Conversation
WalkthroughS3 presigned URL 생성을 포함하도록 음성 분석·전송 흐름이 확장되었습니다. AnalyzeChatService가 음성 파일 검증·WAV 변환·STT·감정분석을 수행하고 WAV를 S3에 업로드해 presigned URL을 생성해 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Analyze as AnalyzeChatService
participant Voice as VoiceService
participant STT as STTService
participant S3 as S3Service
participant Chatbot as ChatbotIntegration
participant SQS as ChatbotSQS
Client->>Analyze: analyze_and_send(file, session_id, user_id, question)
Analyze->>Voice: convert_to_wav(file) (thread/async)
Voice-->>Analyze: wav_bytes
Analyze->>STT: transcribe(wav_bytes)
STT-->>Analyze: transcript, confidence
Analyze->>STT: analyze_emotion(wav_bytes, transcript)
STT-->>Analyze: emotion_payload
Analyze->>S3: _upload_to_s3(wav_bytes, filename...)
S3-->>Analyze: s3_key
Analyze->>S3: get_presigned_url(bucket, s3_key) (optional)
S3-->>Analyze: s3_url
Analyze->>Chatbot: _send_to_chatbot(content, emotion, question, user_id, user_name, session_id, s3_url)
Chatbot->>Chatbot: _create_message_body(..., s3_url)
Chatbot->>SQS: send(payload including s3_url)
SQS-->>Chatbot: ack
Chatbot-->>Analyze: send_result
Analyze-->>Client: final_response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
시
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 0
🧹 Nitpick comments (3)
app/services/chatbot_integration.py (2)
60-66: S3 presigned URL 생성 로직은 적절하며, 실패 시 처리만 약간 다듬어 볼 수 있습니다
S3_BUCKET_NAME과voice.voice_key가 있을 때만 presigned URL 을 생성하는 구조는 안전하게 잘 잡혀 있습니다. 다만get_presigned_url구현상 예외 시""(빈 문자열)을 반환하므로, 현재 코드에서는 이 값이 그대로s3_url필드로 내려갈 수 있습니다. 다운스트림에서 “값 없음”을null/None으로 기대한다면,
s3_url = get_presigned_url(...) or None로 정규화하거나,- URL 생성 실패 시 warning 로그를 추가
하는 식으로 약간만 보완하면 디버깅과 계약이 더 명확해질 것 같습니다.
80-114: 메시지 본문에 s3_url 추가는 일관적이며, 타입 힌트 보강을 고려해 볼 수 있습니다
s3_url을 선택적 필드로 추가해서 SQS 메시지에 함께 실어 보내는 구조는 상단의 presigned URL 생성 로직과 잘 맞습니다. 다만 이 함수는 새 매개변수들에만 타입 힌트가 있고, 반환 타입이 명시되어 있지 않습니다. 팀 컨벤션(“모든 파라미터와 반환 값에 타입 힌트”)에 맞추려면, 추후에라도
- 다른 인자들에 대한 타입 힌트 추가
- 이 함수의 반환 타입(예: dict 형식)에 대한 명시
를 한꺼번에 정리해 두면 IDE/정적 분석기 활용성이 더 좋아질 것입니다.
app/services/analyze_chat_service.py (1)
235-255: _send_to_chatbot의 s3_url 타입 힌트 및 null 전송 방식을 정리하는 것을 권장합니다새로 추가된 시그니처가
s3_url: str = None인데, 이렇게 작성하면 타입 힌트 상으로는None이 허용되지 않는 형태라 mypy/ruff 같은 도구가 경고를 낼 수 있습니다. 리포지토리 가이드라인(“nullable 값에는 Optional[Type] 사용”)과도 맞추기 위해:- session_id: str, - s3_url: str = None # [추가] + session_id: str, + s3_url: Optional[str] = None # [추가]처럼 바꾸고, 상단
from typing import Dict, Any에Optional을 함께 import 하는 방향을 제안드립니다.또한 현재 구현대로면
s3_url이 없을 때 외부 chatbot API 로s3_url: null이 전달될 수 있습니다. 해당 API 가null을 허용하는지, 또는 필드 자체가 생략되기를 기대하는지(예: 필수/선택 여부)가 명확하다면 지금 구조로 충분하지만, 계약이 애매하다면if s3_url is not None:일 때만 payload 에 포함시키는 것도 한 번 검토해 볼 만합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/services/analyze_chat_service.py(5 hunks)app/services/chatbot_integration.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.cursorrules)
**/*.py: Always useDepends(get_db)to inject database sessions into FastAPI endpoints; never usenext(get_db())as it causes connection leaks by preventing the finally block from executing
Never directly instantiate Service or Repository in endpoints; always use FastAPI'sDepends()for dependency injection with dependency functions namedget_{service_name}_deporget_{repo_name}_dep
Never create database sessions directly usingSessionLocal(); sessions must only be created through theget_dbdependency function at the endpoint level
Useasync deffor all FastAPI endpoint functions
Define explicitresponse_modelparameter in FastAPI endpoints using Pydantic models
Organize imports in the following order: (1) standard library, (2) third-party libraries, (3) local modules
Usesnake_casefor endpoint function names,PascalCasefor class names, andUPPER_SNAKE_CASEfor constants
Add docstrings to all functions and complex logic comments in the code
Store sensitive information such as database credentials in environment variables, never hardcode them
Use custom exceptions inheriting fromAppExceptionbase class for consistent error handling and response formatting
Use global exception handlers to catchSQLAlchemyErrorand ensure consistent error response format for database errors
Use type hints on all function parameters and return values
UseOptional[Type]for nullable values and explicitly handleNonecases in code
Use Pydantic models for all request and response data transfer objects (DTOs)
Setautocommit=Falseand explicitly callcommit()for database transactions; callrollback()on exception to ensure data integrity
Use dependency function chaining: Session (top-level) → Service/Repository → endpoint injection
UseAnnotatedwithDependsfor complex dependency scenarios to improve code clarity and reduce repetition
Use streaming for large file processing to avoid excessive memory consumption
Monitor and log memory usage u...
Files:
app/services/analyze_chat_service.pyapp/services/chatbot_integration.py
**/services/**/*.py
📄 CodeRabbit inference engine (.cursorrules)
**/services/**/*.py: Define dependency functions in service and repository files or in a dedicateddependencies.pyfile with explicit type hints for all Service and Repository dependencies
Business logic and Service functions must be located in theservices/directory and receive database sessions through constructor parameters or function parameters, not by direct creation
Files:
app/services/analyze_chat_service.pyapp/services/chatbot_integration.py
🧬 Code graph analysis (1)
app/services/chatbot_integration.py (2)
app/s3_service.py (1)
get_presigned_url(45-57)app/exceptions.py (1)
InternalServerException(37-40)
🪛 Ruff (0.14.6)
app/services/analyze_chat_service.py
243-243: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
app/services/chatbot_integration.py
70-70: Consider moving this statement to an else block
(TRY300)
🔇 Additional comments (1)
app/services/analyze_chat_service.py (1)
54-75: S3 업로드 및 presigned URL 생성 플로우는 자연스럽고, S3 의존성 수준만 한 번 확인하면 좋겠습니다파일을 한 번 메모리에 읽어 두고 S3 업로드/STT/감정 분석에 재사용하는 구조, 그리고
{session_id}/{user_id}/{filename}형태로 키를 만드는 부분은 이해하기 쉽고 일관성 있습니다. 또한bucket과s3_key가 있을 때만 presigned URL 을 생성하도록 한 것도 안전합니다.다만
_upload_to_s3에서S3_BUCKET_NAME이 없으면 곧바로InternalServerException을 던지기 때문에, 현재analyze_and_send경로는 S3 설정이 없으면 무조건 500 으로 실패하게 됩니다. 운영 환경에서 이 API 가 “항상 S3 업로드를 전제”하는 것이 맞다면 괜찮지만, 선택 기능이어야 한다면 chatbot_integration 쪽처럼 S3 미설정 시s3_url없이도 흐름을 계속 가져갈지 정책을 한 번 확인해 보시는 게 좋겠습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/services/chatbot_integration.py (1)
67-76: try-except-else 패턴 사용을 고려하세요.정상 흐름의 return 문(Line 70)을
else블록으로 이동하면 예외 처리 구조가 더 명확해집니다.다음 diff를 참고하세요:
- response = _send_sqs_message(sqs_url, message_body) - - logger.info(f"[ChatbotIntegration] Sent to chatbot SQS. MsgID={response.get('MessageId')}, VoiceID={voice_id}") - return {"message": f"Analysis event for voice_id {voice_id} sent successfully."} - except Exception as e: logger.error(f"[ChatbotIntegration] Failed to process or send SQS: {e}", exc_info=True) raise InternalServerException( message=f"An unexpected error occurred during SQS preparation or transmission: {type(e).__name__}" ) from e + else: + response = _send_sqs_message(sqs_url, message_body) + + logger.info(f"[ChatbotIntegration] Sent to chatbot SQS. MsgID={response.get('MessageId')}, VoiceID={voice_id}") + return {"message": f"Analysis event for voice_id {voice_id} sent successfully."}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/services/analyze_chat_service.py(5 hunks)app/services/chatbot_integration.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.cursorrules)
**/*.py: Always useDepends(get_db)to inject database sessions into FastAPI endpoints; never usenext(get_db())as it causes connection leaks by preventing the finally block from executing
Never directly instantiate Service or Repository in endpoints; always use FastAPI'sDepends()for dependency injection with dependency functions namedget_{service_name}_deporget_{repo_name}_dep
Never create database sessions directly usingSessionLocal(); sessions must only be created through theget_dbdependency function at the endpoint level
Useasync deffor all FastAPI endpoint functions
Define explicitresponse_modelparameter in FastAPI endpoints using Pydantic models
Organize imports in the following order: (1) standard library, (2) third-party libraries, (3) local modules
Usesnake_casefor endpoint function names,PascalCasefor class names, andUPPER_SNAKE_CASEfor constants
Add docstrings to all functions and complex logic comments in the code
Store sensitive information such as database credentials in environment variables, never hardcode them
Use custom exceptions inheriting fromAppExceptionbase class for consistent error handling and response formatting
Use global exception handlers to catchSQLAlchemyErrorand ensure consistent error response format for database errors
Use type hints on all function parameters and return values
UseOptional[Type]for nullable values and explicitly handleNonecases in code
Use Pydantic models for all request and response data transfer objects (DTOs)
Setautocommit=Falseand explicitly callcommit()for database transactions; callrollback()on exception to ensure data integrity
Use dependency function chaining: Session (top-level) → Service/Repository → endpoint injection
UseAnnotatedwithDependsfor complex dependency scenarios to improve code clarity and reduce repetition
Use streaming for large file processing to avoid excessive memory consumption
Monitor and log memory usage u...
Files:
app/services/chatbot_integration.pyapp/services/analyze_chat_service.py
**/services/**/*.py
📄 CodeRabbit inference engine (.cursorrules)
**/services/**/*.py: Define dependency functions in service and repository files or in a dedicateddependencies.pyfile with explicit type hints for all Service and Repository dependencies
Business logic and Service functions must be located in theservices/directory and receive database sessions through constructor parameters or function parameters, not by direct creation
Files:
app/services/chatbot_integration.pyapp/services/analyze_chat_service.py
🧬 Code graph analysis (1)
app/services/chatbot_integration.py (2)
app/s3_service.py (1)
get_presigned_url(45-57)app/exceptions.py (1)
InternalServerException(37-40)
🪛 Ruff (0.14.6)
app/services/chatbot_integration.py
70-70: Consider moving this statement to an else block
(TRY300)
app/services/analyze_chat_service.py
243-243: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
🔇 Additional comments (6)
app/services/analyze_chat_service.py (3)
15-15: LGTM!
get_presigned_url임포트가 적절하게 추가되었으며, 이후 코드에서 올바르게 사용되고 있습니다.
247-255: LGTM!챗봇 API 요청 페이로드에
s3_url필드가 올바르게 추가되었습니다. 기존 필드들과 함께 적절하게 구성되어 있습니다.
61-105: 처리 흐름이 적절합니다.S3 업로드 → 프리사인드 URL 생성 → STT → 감정 분석 → 챗봇 전송 순서가 논리적이며,
s3_url이 선택적으로 처리되어 견고합니다.app/services/chatbot_integration.py (3)
15-15: LGTM!
get_presigned_url임포트가 적절하게 추가되었습니다.
80-114: LGTM!
_create_message_body함수에s3_url: Optional[str]파라미터가 올바르게 추가되었으며, SQS 메시지 본문에 적절하게 포함되어 있습니다. 타입 힌트도 명시적으로 잘 작성되었습니다.
60-64: Presigned URL generation logic is correct and verified.Voice model contains the
voice_keyfield (String(1024), non-nullable), and the implementation correctly generates presigned URLs with 14-day expiry. The defensive checks forbucketandvoice.voice_keyare appropriate. Implementation is consistent withanalyze_chat_service.py(line 74) which uses identical 14-day expiry logic.
| user_name: str, | ||
| session_id: str | ||
| session_id: str, | ||
| s3_url: str = None # [추가] |
There was a problem hiding this comment.
타입 힌트를 명시적으로 작성하세요.
PEP 484에 따르면 str = None은 암묵적 Optional로 권장되지 않습니다. 명시적으로 Optional[str] = None을 사용하세요.
다음 diff를 적용하여 수정하세요:
- s3_url: str = None # [추가]
+ s3_url: Optional[str] = None # [추가]📝 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.
| s3_url: str = None # [추가] | |
| s3_url: Optional[str] = None # [추가] |
🧰 Tools
🪛 Ruff (0.14.6)
243-243: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
🤖 Prompt for AI Agents
In app/services/analyze_chat_service.py around line 243, the variable
declaration uses an implicit optional type "s3_url: str = None" which violates
PEP 484; change the annotation to "s3_url: Optional[str] = None" and add "from
typing import Optional" at the top of the file if not already imported so the
Optional type is available.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
app/services/analyze_chat_service.py (2)
70-75: Presigned URL 만료 시간을 더 짧게/설정값으로 조정하는 것을 고려해주세요.현재
expires_in=3600 * 24 * 7로 최대 7일짜리 URL을 발급하고 있습니다. 이 URL은 업로드 직후 챗봇 API에 함께 전달되는 용도라, 실제로 7일 동안 유효할 필요가 있는지 한번 더 확인하는 게 좋습니다.
보안 관점에서는 필요한 기간보다 긴 presigned URL은 노출 시 피해 범위를 키우는 요소가 되므로, 아래와 같이 기본값(1시간) 사용이나 환경변수 기반 TTL로 줄이는 옵션을 검토해 보시는 걸 권장합니다.- if bucket and s3_key: - s3_url = get_presigned_url(bucket, s3_key, expires_in=3600 * 24 * 7) + if bucket and s3_key: + # 필요 시 환경변수 등으로 TTL을 조정하는 것도 고려 + s3_url = get_presigned_url(bucket, s3_key) # 기본 1시간 (3600초)
4-4:s3_url파라미터를 명시적으로Optional[str]로 선언해주세요.현재는
s3_url: str = None형태라 implicit Optional(PEP 484 비권장) 패턴이고, 저장소 코딩 가이드에서도 nullable 값에는Optional[Type]사용을 요구하고 있습니다. 아래처럼 타입 힌트를 명시적으로 바꾸는 것을 추천합니다.-from typing import Dict, Any +from typing import Dict, Any, Optional @@ - user_id: str, - user_name: str, - session_id: str, - s3_url: str = None # [추가] + user_id: str, + user_name: str, + session_id: str, + s3_url: Optional[str] = None # [추가]이렇게 하면 Ruff의 RUF013 경고도 함께 해소됩니다.
Also applies to: 235-244
app/services/chatbot_integration.py (1)
60-66: SQS 플로우용 presigned TTL은 상수/환경변수로 분리하는 것을 고려해보세요.SQS 소비자가 지연 처리될 수 있다는 점에서
3600 * 24 * 7(7일) TTL을 주는 것은 이해됩니다. 다만 같은 TTL 값을 여기저기서 직접 써버리면 향후 정책 변경 시 관리가 어렵습니다.예를 들어 다음처럼 상수/환경변수로 분리해두면 관리가 쉬워집니다.
# 모듈 상단 등 CHATBOT_S3_PRESIGNED_TTL = int(os.getenv("CHATBOT_S3_PRESIGNED_TTL", str(3600 * 24 * 7)))- if bucket and voice.voice_key: - s3_url = get_presigned_url(bucket, voice.voice_key, expires_in=3600 * 24 * 7) + if bucket and voice.voice_key: + s3_url = get_presigned_url(bucket, voice.voice_key, expires_in=CHATBOT_S3_PRESIGNED_TTL)
🧹 Nitpick comments (1)
app/services/chatbot_integration.py (1)
107-114: 메시지 바디에s3_url및 메타데이터를 추가한 구조가 일관적입니다.
user_name,question,content,recorded_at등 사람이 읽기 좋은 맥락과 함께s3_url과emotion, 전송 시각(timestamp)까지 포함되어 있어 다운스트림 챗봇/분석 시스템에서 재처리하기 좋습니다.한 가지 참고사항으로, 장기적으로는
timestamp를datetime.now(timezone.utc).isoformat()형태의 timezone-aware 값으로 바꾸는 것도 고려할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/services/analyze_chat_service.py(5 hunks)app/services/chatbot_integration.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.cursorrules)
**/*.py: Always useDepends(get_db)to inject database sessions into FastAPI endpoints; never usenext(get_db())as it causes connection leaks by preventing the finally block from executing
Never directly instantiate Service or Repository in endpoints; always use FastAPI'sDepends()for dependency injection with dependency functions namedget_{service_name}_deporget_{repo_name}_dep
Never create database sessions directly usingSessionLocal(); sessions must only be created through theget_dbdependency function at the endpoint level
Useasync deffor all FastAPI endpoint functions
Define explicitresponse_modelparameter in FastAPI endpoints using Pydantic models
Organize imports in the following order: (1) standard library, (2) third-party libraries, (3) local modules
Usesnake_casefor endpoint function names,PascalCasefor class names, andUPPER_SNAKE_CASEfor constants
Add docstrings to all functions and complex logic comments in the code
Store sensitive information such as database credentials in environment variables, never hardcode them
Use custom exceptions inheriting fromAppExceptionbase class for consistent error handling and response formatting
Use global exception handlers to catchSQLAlchemyErrorand ensure consistent error response format for database errors
Use type hints on all function parameters and return values
UseOptional[Type]for nullable values and explicitly handleNonecases in code
Use Pydantic models for all request and response data transfer objects (DTOs)
Setautocommit=Falseand explicitly callcommit()for database transactions; callrollback()on exception to ensure data integrity
Use dependency function chaining: Session (top-level) → Service/Repository → endpoint injection
UseAnnotatedwithDependsfor complex dependency scenarios to improve code clarity and reduce repetition
Use streaming for large file processing to avoid excessive memory consumption
Monitor and log memory usage u...
Files:
app/services/analyze_chat_service.pyapp/services/chatbot_integration.py
**/services/**/*.py
📄 CodeRabbit inference engine (.cursorrules)
**/services/**/*.py: Define dependency functions in service and repository files or in a dedicateddependencies.pyfile with explicit type hints for all Service and Repository dependencies
Business logic and Service functions must be located in theservices/directory and receive database sessions through constructor parameters or function parameters, not by direct creation
Files:
app/services/analyze_chat_service.pyapp/services/chatbot_integration.py
🧠 Learnings (1)
📚 Learning: 2025-11-25T12:27:58.712Z
Learnt from: CR
Repo: safori-team/CARING-Back PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T12:27:58.712Z
Learning: Applies to **/*.py : Use `Optional[Type]` for nullable values and explicitly handle `None` cases in code
Applied to files:
app/services/analyze_chat_service.py
🧬 Code graph analysis (2)
app/services/analyze_chat_service.py (1)
app/s3_service.py (2)
upload_fileobj(26-32)get_presigned_url(45-57)
app/services/chatbot_integration.py (2)
app/s3_service.py (1)
get_presigned_url(45-57)app/exceptions.py (1)
InternalServerException(37-40)
🪛 Ruff (0.14.6)
app/services/analyze_chat_service.py
243-243: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
app/services/chatbot_integration.py
70-70: Consider moving this statement to an else block
(TRY300)
🔇 Additional comments (8)
app/services/analyze_chat_service.py (6)
15-15: S3 서비스 의존성 import 추가는 적절합니다.
upload_fileobj,get_presigned_url를 이 서비스에서 직접 사용하도록 분리한 구조가 명확하고, 다른 모듈에 대한 결합도도 적절한 수준입니다.
54-59: 파일명 유니크 처리 방식이 명확하고 안전합니다.현재 시각 + 8자리 UUID를 suffix로 붙여 충돌 가능성을 충분히 낮췄고, 원래 파일명/확장자를 유지해 디버깅에도 도움이 됩니다.
61-68: S3 업로드를asyncio.to_thread로 분리한 구조가 적절합니다.동기식 S3 업로드를 워커 스레드로 분리해 이벤트 루프 블로킹을 피한 점이 좋습니다. S3 키 패턴(
{session_id}/{user_id}/{filename}또는chat/)도 나중에 추적하기에 이해하기 쉽습니다.
76-105: STT/감정 분석 후s3_url까지 포함해 챗봇으로 보내는 플로우가 자연스럽습니다.
- STT → 감정 분석 → 사용자 정보 조회 →
_send_to_chatbot호출 순서가 명확합니다.s3_url이None일 수 있는 상황에서도 그대로 전달하므로, 외부 챗봇 API가null/미존재 둘 다를 허용하는지 계약만 한번 더 확인하면 될 것 같습니다.
171-172: 단계 번호가 있는 주석이 로직 이해에 도움 됩니다.
3-1. Audio 감정 분석처럼 단계별로 주석을 나눠 둔 덕분에 이후 유지보수 시 각 서브단계(3‑1, 3‑2, 3‑3)를 추적하기 수월해 보입니다.
248-255: 챗봇 요청 바디에emotion/s3_url를 포함하는 변경은 일관적입니다.
emotion전체 구조를 그대로 전달하고,s3_url을 별도 필드로 추가해 챗봇 쪽에서 원본 오디오 접근이 가능해졌습니다.외부 챗봇 API 스펙(필드명, null 허용 여부)이 이미 이 형태로 합의되어 있는지만 한번 더 확인해 주세요.
app/services/chatbot_integration.py (2)
15-15: S3 presigned URL 유틸리티 import 추가는 자연스러운 확장입니다.SQS 전송 전에 S3 오브젝트에 대한 접근 URL을 생성하는 목적에 맞게
get_presigned_url을 재사용한 구조가 좋습니다.
80-80:_create_message_body시그니처에s3_url: Optional[str]추가 설계가 깔끔합니다.S3 URL이 없는 경우도 자연스럽게 처리할 수 있고, 호출 측에서 타입을 명확히 알 수 있어 유지보수성이 좋아졌습니다.
fix : analyze voice STT 오류 해결
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
app/services/analyze_chat_service.py (2)
106-110: Presigned URL 만료 시간 검토 필요이전 리뷰에서 14일에서 1시간으로 줄이라고 제안했는데, 현재 7일(
3600 * 24 * 7)로 설정되어 있습니다. 챗봇 API가 URL을 즉시 소비하는 경우 1시간이면 충분할 수 있습니다. 챗봇 서비스가 URL을 지연 처리해야 하는 요구사항이 있다면 현재 값을 유지하되, 그렇지 않다면 보안을 위해 만료 시간을 줄이는 것을 권장합니다.챗봇 API가 presigned URL을 언제 사용하는지 확인해 주세요. 즉시 사용한다면
expires_in=3600으로 줄이는 것이 좋습니다.
281-282:Optional[str]타입 힌트 사용 필요PEP 484에 따르면
str = None은 암묵적 Optional으로 권장되지 않습니다. 코딩 가이드라인에 따라 명시적으로Optional[str]을 사용하세요.- s3_url: str = None # [추가] + s3_url: Optional[str] = None # [추가]또한 파일 상단의 import 문에
Optional을 추가해야 합니다:-from typing import Dict, Any +from typing import Dict, Any, Optional
🧹 Nitpick comments (4)
app/stt_service.py (1)
159-187: LGTM! 다중 결과 처리 개선모든 STT 결과를 합쳐 전체 텍스트를 생성하고 평균 confidence를 계산하는 방식이 긴 오디오 파일 처리에 적합합니다. 빈 confidence 목록에 대한 처리도 잘 되어 있습니다.
Line 172의 TODO 주석(낮은 confidence 결과 제외 로직)을 구현하시겠습니까? 필요하시면 이슈를 생성해 드릴 수 있습니다.
app/services/analyze_chat_service.py (3)
92-95: private 메서드 접근 리팩터링 권장
self.voice_service._convert_to_wav는 private 메서드(언더스코어 접두사)를 직접 호출합니다. 이는 캡슐화를 위반하고VoiceService의 내부 구현에 강하게 결합됩니다.
VoiceService에 public 래퍼 메서드를 추가하는 것을 권장합니다:# voice_service.py에 추가 def convert_to_wav(self, file_content: bytes, original_filename: str) -> Tuple[bytes, str]: """Convert audio file to WAV format (public API)""" return self._convert_to_wav(file_content, original_filename)그런 다음 이 파일에서 호출:
- wav_content, wav_filename = await asyncio.to_thread( - self.voice_service._convert_to_wav, file_content, filename - ) + wav_content, wav_filename = await asyncio.to_thread( + self.voice_service.convert_to_wav, file_content, filename + )
71-71: Pythonic 멤버십 테스트 사용 권장
'.' in filename의 부정은'.' not in filename으로 작성하는 것이 더 Pythonic합니다.- if not '.' in filename or (not lower_name.endswith('.wav') and not lower_name.endswith('.m4a')): + if '.' not in filename or (not lower_name.endswith('.wav') and not lower_name.endswith('.m4a')):
172-174: INFO 레벨 대신 WARNING 사용은 안티패턴주석에 "INFO 레벨이 출력되지 않을 수 있으므로 WARNING으로 남긴다"고 되어 있지만, 이는 로깅 구성 문제를 우회하는 것입니다. WARNING 레벨을 정보성 로그에 사용하면 실제 경고를 찾기 어려워집니다.
로깅 구성에서 INFO 레벨을 활성화하거나, 개발 중에만 DEBUG 레벨을 사용하세요:
- # INFO 레벨은 기본 로거 설정에 따라 출력되지 않을 수 있으므로 WARNING으로 남겨 확실히 보이게 한다. - logger.warning("AnalyzeChat STT 시작 - filename=%s, content_type=%s", filename, content_type) + logger.info("AnalyzeChat STT 시작 - filename=%s, content_type=%s", filename, content_type)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/services/analyze_chat_service.py(7 hunks)app/stt_service.py(3 hunks)app/voice_service.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.cursorrules)
**/*.py: Always useDepends(get_db)to inject database sessions into FastAPI endpoints; never usenext(get_db())as it causes connection leaks by preventing the finally block from executing
Never directly instantiate Service or Repository in endpoints; always use FastAPI'sDepends()for dependency injection with dependency functions namedget_{service_name}_deporget_{repo_name}_dep
Never create database sessions directly usingSessionLocal(); sessions must only be created through theget_dbdependency function at the endpoint level
Useasync deffor all FastAPI endpoint functions
Define explicitresponse_modelparameter in FastAPI endpoints using Pydantic models
Organize imports in the following order: (1) standard library, (2) third-party libraries, (3) local modules
Usesnake_casefor endpoint function names,PascalCasefor class names, andUPPER_SNAKE_CASEfor constants
Add docstrings to all functions and complex logic comments in the code
Store sensitive information such as database credentials in environment variables, never hardcode them
Use custom exceptions inheriting fromAppExceptionbase class for consistent error handling and response formatting
Use global exception handlers to catchSQLAlchemyErrorand ensure consistent error response format for database errors
Use type hints on all function parameters and return values
UseOptional[Type]for nullable values and explicitly handleNonecases in code
Use Pydantic models for all request and response data transfer objects (DTOs)
Setautocommit=Falseand explicitly callcommit()for database transactions; callrollback()on exception to ensure data integrity
Use dependency function chaining: Session (top-level) → Service/Repository → endpoint injection
UseAnnotatedwithDependsfor complex dependency scenarios to improve code clarity and reduce repetition
Use streaming for large file processing to avoid excessive memory consumption
Monitor and log memory usage u...
Files:
app/voice_service.pyapp/services/analyze_chat_service.pyapp/stt_service.py
**/services/**/*.py
📄 CodeRabbit inference engine (.cursorrules)
**/services/**/*.py: Define dependency functions in service and repository files or in a dedicateddependencies.pyfile with explicit type hints for all Service and Repository dependencies
Business logic and Service functions must be located in theservices/directory and receive database sessions through constructor parameters or function parameters, not by direct creation
Files:
app/services/analyze_chat_service.py
🧠 Learnings (1)
📚 Learning: 2025-11-25T12:27:58.712Z
Learnt from: CR
Repo: safori-team/CARING-Back PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T12:27:58.712Z
Learning: Applies to **/*.py : Use `Optional[Type]` for nullable values and explicitly handle `None` cases in code
Applied to files:
app/services/analyze_chat_service.py
🧬 Code graph analysis (1)
app/stt_service.py (1)
app/emotion_service.py (1)
robust_load(74-96)
🪛 Ruff (0.14.6)
app/voice_service.py
57-57: f-string without any placeholders
Remove extraneous f prefix
(F541)
app/services/analyze_chat_service.py
71-71: Test for membership should be not in
Convert to not in
(E713)
282-282: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
app/stt_service.py
101-101: Do not catch blind exception: Exception
(BLE001)
199-199: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (6)
app/voice_service.py (2)
77-83: LGTM! 임시 파일 무결성 검증 개선
fsync와 쓰기 크기 검증을 추가하여 임시 파일이 제대로 생성되었는지 확인하는 것은 좋은 방어적 프로그래밍입니다. 오디오 처리 파이프라인의 안정성을 높여줍니다.
46-58: LGTM! M4A 헤더 검증 로깅 추가M4A/MP4 파일의
ftypatom과moovbox 존재 여부를 로깅하여 손상된 파일을 조기에 감지할 수 있습니다. 비차단 방식으로 경고만 출력하고 실제 변환은 ffmpeg/librosa에 위임하는 것이 적절합니다.app/stt_service.py (3)
88-114: LGTM! robust_load 패턴 일관성 유지
soundfile우선,librosa폴백 방식의 로더가emotion_service.py의 패턴과 일치합니다. 다양한 오디오 포맷에 대한 견고한 처리를 제공합니다.
203-214: LGTM! 임시 파일 정리 로직 개선
finally블록에서 임시 파일 삭제 실패 시 경고만 로깅하고 예외를 발생시키지 않아 원래 예외가 가려지는 것을 방지합니다.
217-223: LGTM! 전역 인스턴스 및 편의 함수
GoogleSTTService는 데이터베이스 접근이 필요 없는 외부 API 클라이언트이므로 전역 싱글톤 패턴이 적절합니다.transcribe_voice함수는 다른 모듈에서 쉽게 사용할 수 있는 간단한 API를 제공합니다.app/services/analyze_chat_service.py (1)
38-141: LGTM! analyze_and_send 흐름 구현WAV 변환 → S3 업로드 → presigned URL 생성 → STT → 감정 분석 → 챗봇 전송의 전체 흐름이 잘 구성되어 있습니다. CPU 집약적 작업에
asyncio.to_thread를 사용하여 블로킹을 방지하는 것이 좋습니다.s3_url필드가 챗봇 페이로드에 성공적으로 추가되었습니다.
| if not has_ftyp: | ||
| print(f"[convert] WARNING: M4A 파일이지만 ftyp atom이 없음, 손상된 파일일 수 있음") |
There was a problem hiding this comment.
불필요한 f-string 접두사 제거 필요
Line 57에서 f-string이 사용되었지만 플레이스홀더가 없습니다. 일반 문자열로 변경하세요.
- print(f"[convert] WARNING: M4A 파일이지만 ftyp atom이 없음, 손상된 파일일 수 있음")
+ print("[convert] WARNING: M4A 파일이지만 ftyp atom이 없음, 손상된 파일일 수 있음")📝 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.
| if not has_ftyp: | |
| print(f"[convert] WARNING: M4A 파일이지만 ftyp atom이 없음, 손상된 파일일 수 있음") | |
| if not has_ftyp: | |
| print("[convert] WARNING: M4A 파일이지만 ftyp atom이 없음, 손상된 파일일 수 있음") |
🧰 Tools
🪛 Ruff (0.14.6)
57-57: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
In app/voice_service.py around lines 56 to 57, the print statement uses an
f-string without placeholders; change the f-string to a normal string literal by
removing the leading f so it becomes print("[convert] WARNING: M4A 파일이지만 ftyp
atom이 없음, 손상된 파일일 수 있음").
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
app/voice_service.py (1)
57-57: 불필요한 f-string 접두사를 제거하세요.Line 57에서 f-string이 사용되었지만 플레이스홀더가 없습니다. 일반 문자열로 변경하세요.
다음 diff를 적용하세요:
- print(f"[convert] WARNING: M4A 파일이지만 ftyp atom이 없음, 손상된 파일일 수 있음") + print("[convert] WARNING: M4A 파일이지만 ftyp atom이 없음, 손상된 파일일 수 있음")app/services/analyze_chat_service.py (1)
282-282: 타입 힌트를 명시적으로 작성하세요.PEP 484에 따르면
str = None은 암묵적 Optional로 권장되지 않습니다. 명시적으로Optional[str] = None을 사용하세요.다음 diff를 적용하세요:
- s3_url: str = None # [추가] + s3_url: Optional[str] = None # [추가]파일 상단에
Optional을 import해야 합니다 (이미 import되어 있는지 확인):from typing import Dict, Any, Optional
🧹 Nitpick comments (1)
app/services/va_fusion.py (1)
279-290: 예외 처리 시 로깅 추가를 고려하세요.angry 감쇠 로직이 실패할 때 조용히 무시하도록 설계되었지만, 디버깅과 모니터링을 위해 최소한의 로깅을 추가하는 것이 좋습니다.
다음 diff를 적용하여 로깅을 추가할 수 있습니다:
try: angry_p = float(audio_probs.get("angry", 0.0)) if angry_p <= 0.2: neg = max(0.0, -float(v_text)) mag = max(0.0, min(1.0, float(a_text))) base_factor = 0.7 extra_down = 0.15 * neg * mag # 최대 약 0.15 추가 감쇠 factor = max(0.5, base_factor - extra_down) # 최소 0.5배까지 composite_score["angry"] = composite_score.get("angry", 0.0) * factor - except Exception: + except Exception as e: # 로직 실패 시에는 안전하게 무시 + import logging + logging.getLogger(__name__).debug(f"Angry damping failed (ignored): {e}") pass
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/main.py(1 hunks)app/services/analyze_chat_service.py(8 hunks)app/services/va_fusion.py(2 hunks)app/voice_service.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.cursorrules)
**/*.py: Always useDepends(get_db)to inject database sessions into FastAPI endpoints; never usenext(get_db())as it causes connection leaks by preventing the finally block from executing
Never directly instantiate Service or Repository in endpoints; always use FastAPI'sDepends()for dependency injection with dependency functions namedget_{service_name}_deporget_{repo_name}_dep
Never create database sessions directly usingSessionLocal(); sessions must only be created through theget_dbdependency function at the endpoint level
Useasync deffor all FastAPI endpoint functions
Define explicitresponse_modelparameter in FastAPI endpoints using Pydantic models
Organize imports in the following order: (1) standard library, (2) third-party libraries, (3) local modules
Usesnake_casefor endpoint function names,PascalCasefor class names, andUPPER_SNAKE_CASEfor constants
Add docstrings to all functions and complex logic comments in the code
Store sensitive information such as database credentials in environment variables, never hardcode them
Use custom exceptions inheriting fromAppExceptionbase class for consistent error handling and response formatting
Use global exception handlers to catchSQLAlchemyErrorand ensure consistent error response format for database errors
Use type hints on all function parameters and return values
UseOptional[Type]for nullable values and explicitly handleNonecases in code
Use Pydantic models for all request and response data transfer objects (DTOs)
Setautocommit=Falseand explicitly callcommit()for database transactions; callrollback()on exception to ensure data integrity
Use dependency function chaining: Session (top-level) → Service/Repository → endpoint injection
UseAnnotatedwithDependsfor complex dependency scenarios to improve code clarity and reduce repetition
Use streaming for large file processing to avoid excessive memory consumption
Monitor and log memory usage u...
Files:
app/main.pyapp/services/va_fusion.pyapp/services/analyze_chat_service.pyapp/voice_service.py
**/services/**/*.py
📄 CodeRabbit inference engine (.cursorrules)
**/services/**/*.py: Define dependency functions in service and repository files or in a dedicateddependencies.pyfile with explicit type hints for all Service and Repository dependencies
Business logic and Service functions must be located in theservices/directory and receive database sessions through constructor parameters or function parameters, not by direct creation
Files:
app/services/va_fusion.pyapp/services/analyze_chat_service.py
🧠 Learnings (1)
📚 Learning: 2025-11-25T12:27:58.712Z
Learnt from: CR
Repo: safori-team/CARING-Back PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T12:27:58.712Z
Learning: Applies to **/*.py : Use `Optional[Type]` for nullable values and explicitly handle `None` cases in code
Applied to files:
app/services/analyze_chat_service.py
🧬 Code graph analysis (2)
app/main.py (1)
app/voice_service.py (1)
to_bps(446-450)
app/services/analyze_chat_service.py (3)
app/s3_service.py (1)
get_presigned_url(45-57)app/voice_service.py (1)
get_voice_service(953-955)app/main.py (1)
FileWrapper(915-919)
🪛 Ruff (0.14.6)
app/services/va_fusion.py
288-290: try-except-pass detected, consider logging the exception
(S110)
288-288: Do not catch blind exception: Exception
(BLE001)
app/services/analyze_chat_service.py
71-71: Test for membership should be not in
Convert to not in
(E713)
282-282: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
app/voice_service.py
57-57: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (5)
app/main.py (1)
932-937: LGTM! fear/anxiety 통합 로직이 정확합니다.Preview API에서도 voice_service.py와 동일하게 fear와 anxiety를 단일 버킷으로 통합하여 일관성을 유지하고 있습니다.
app/voice_service.py (2)
46-56: LGTM! WAV 변환 과정의 가시성이 향상되었습니다.파일 크기 로깅, M4A 헤더 검증, fsync를 통한 디스크 동기화 등의 개선사항이 디버깅과 파일 무결성 검증에 도움이 됩니다.
Also applies to: 77-83
457-464: LGTM! fear/anxiety/fearful 통합 로직이 명확합니다.감정 범주를 단일 버킷으로 통합하는 의도가 주석에 잘 설명되어 있으며, 구현도 정확합니다.
app/services/analyze_chat_service.py (2)
77-95: LGTM! VoiceService와의 통합이 적절합니다.파일 읽기, 크기 검증, WAV 변환 로직이 voice_service를 재사용하여 코드 중복을 방지하고 있습니다.
113-127: LGTM! STT 및 감정 분석 파이프라인이 적절합니다.WAV 데이터를 사용한 STT와 감정 분석 처리가 올바르게 구현되어 있으며, 적절한 예외 처리가 포함되어 있습니다.
| "message": "Only wav/m4a audio is allowed" | ||
| } | ||
| # 확장자 미포함/이상치인 경우 Content-Type 기반으로 보정 | ||
| if not '.' in filename or (not lower_name.endswith('.wav') and not lower_name.endswith('.m4a')): |
There was a problem hiding this comment.
not in 연산자를 사용하세요.
가독성과 PEP 8 준수를 위해 not '.' in filename 대신 '.' not in filename을 사용하세요.
다음 diff를 적용하세요:
# 확장자 미포함/이상치인 경우 Content-Type 기반으로 보정
- if not '.' in filename or (not lower_name.endswith('.wav') and not lower_name.endswith('.m4a')):
+ if '.' not in filename or (not lower_name.endswith('.wav') and not lower_name.endswith('.m4a')):
if "m4a" in content_type or "mp4" in content_type:📝 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.
| if not '.' in filename or (not lower_name.endswith('.wav') and not lower_name.endswith('.m4a')): | |
| if '.' not in filename or (not lower_name.endswith('.wav') and not lower_name.endswith('.m4a')): | |
| if "m4a" in content_type or "mp4" in content_type: |
🧰 Tools
🪛 Ruff (0.14.6)
71-71: Test for membership should be not in
Convert to not in
(E713)
🤖 Prompt for AI Agents
In app/services/analyze_chat_service.py around line 71, the conditional uses the
non-PEP8 form "not '.' in filename"; replace that with the PEP8-compliant "'.'
not in filename" so the line reads: if '.' not in filename or (not
lower_name.endswith('.wav') and not lower_name.endswith('.m4a')) — keep the rest
of the logic unchanged.
2 participants
🔎 Description
🔗 Related Issue
🏷️ What type of PR is this?
📋 Changes Made
🧪 Testing
📸 Screenshots (if applicable)
📝 Additional Notes
Summary by CodeRabbit
새로운 기능
버그 수정 / 안정성
✏️ Tip: You can customize this high-level summary in your review settings.