Merged
Conversation
…and observer cleanup
…-deps, and update docs
…ling, metadata update fix)
… of PR feedback fixes
… (deploy.yml, Dockerfile, ingest_data.py, update_metadata.py)
…feedback implementation
…in Dockerfile and correcting port binding
…le exec, fail-fast config, .dockerignore cleanup)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthrough배포 대상이 Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as "Developer"
participant CI as "CI / GitHub Actions"
participant Frontend as "Firebase Hosting"
participant Backend as "FastAPI App"
participant Supabase as "Supabase (env)"
rect rgba(200,230,255,0.5)
Dev->>CI: push 변경 (firebase.json, main.py, Dockerfile)
CI->>Frontend: 배포 대상 = philo-rag
end
rect rgba(200,255,200,0.5)
Frontend->>Backend: 사용자 요청 (브라우저)
Backend->>Backend: lifespan 시작 -> validate_required_settings()
Backend->>Supabase: 환경변수(SUPABASE_*) 존재 확인
Backend->>Backend: CORS 미들웨어 초기화 (명시적 allowlist)
Backend->>Frontend: 응답 (CORS 허용 검증, /ready 엔드포인트 체크)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
시
🚥 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 docstrings
🧪 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@analysis_results.md`:
- Line 39: The document mixes languages: Line "2. **Re-run the GitHub Action**:
PR을 `main` 브랜치에 머지(merge)하면 GitHub Actions 워크플로우가 트리거됩니다." should be made
consistent with the rest of the file; replace the Korean portion with an English
equivalent (e.g., "When you merge the PR into the main branch, the GitHub
Actions workflow is triggered.") so the entire document uses a single language;
ensure punctuation and formatting (bold/inline code) remain unchanged.
In `@backend/app/core/config.py`:
- Around line 24-30: 현재 config.py에서 settings = Settings() 직후에 모듈 import 시점에
RuntimeError를 발생시키는 fail-fast 검증이 있어 테스트/도구 실행 중 import 단계에서 실패합니다; 대신
config.py에 validate_required_settings()라는 함수(검증 로직을 그대로 포함,
SUPABASE_URL/SUPABASE_SERVICE_KEY 체크 및 RuntimeError 발생)를 추가하고 모듈 레벨에서는 예외를 던지지
않도록 단순히 settings = Settings()만 유지한 뒤, 애플리케이션 시작 시점(예: main.py의 startup/lifespan
이벤트 핸들러)에서 validate_required_settings()를 호출하여 fail-fast 동작은 유지하되 import-time
side effect를 제거하세요.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 27c31ae7-4de0-4778-ad1d-d25ead6f44ea
📒 Files selected for processing (6)
analysis_results.mdbackend/.dockerignorebackend/Dockerfilebackend/app/core/config.pybackend/app/main.pyfrontend/firebase.json
Comment on lines
+24
to
+30
| settings = Settings() | ||
|
|
||
| # Fail-fast validation: ensure essential secrets are configured at startup | ||
| if not settings.SUPABASE_URL or not settings.SUPABASE_SERVICE_KEY: | ||
| raise RuntimeError( | ||
| "SUPABASE_URL and SUPABASE_SERVICE_KEY (or SUPABASE_SERVICE_ROLE_KEY) must be configured in environment variables." | ||
| ) |
There was a problem hiding this comment.
Import 시점 예외 발생으로 테스트/도구 실행이 깨질 수 있습니다.
Line 24-30의 fail-fast가 모듈 import 단계에서 즉시 RuntimeError를 발생시켜, backend/tests/conftest.py의 fixture 적용 전에 테스트 수집이 실패할 수 있습니다(관련: backend/app/services/database.py Line 1-5의 모듈 레벨 import). 검증은 앱 시작 단계(예: lifespan/startup)로 이동하는 편이 안전합니다.
🔧 제안 수정안
settings = Settings()
-# Fail-fast validation: ensure essential secrets are configured at startup
-if not settings.SUPABASE_URL or not settings.SUPABASE_SERVICE_KEY:
- raise RuntimeError(
- "SUPABASE_URL and SUPABASE_SERVICE_KEY (or SUPABASE_SERVICE_ROLE_KEY) must be configured in environment variables."
- )
+def validate_required_settings() -> None:
+ if not settings.SUPABASE_URL or not settings.SUPABASE_SERVICE_KEY:
+ raise RuntimeError(
+ "SUPABASE_URL and SUPABASE_SERVICE_KEY (or SUPABASE_SERVICE_ROLE_KEY) must be configured in environment variables."
+ )그리고 backend/app/main.py의 startup/lifespan에서 validate_required_settings()를 호출해 fail-fast 성질은 유지하되 import-time side effect를 제거하는 구성이 좋습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/core/config.py` around lines 24 - 30, 현재 config.py에서 settings =
Settings() 직후에 모듈 import 시점에 RuntimeError를 발생시키는 fail-fast 검증이 있어 테스트/도구 실행 중
import 단계에서 실패합니다; 대신 config.py에 validate_required_settings()라는 함수(검증 로직을 그대로
포함, SUPABASE_URL/SUPABASE_SERVICE_KEY 체크 및 RuntimeError 발생)를 추가하고 모듈 레벨에서는 예외를
던지지 않도록 단순히 settings = Settings()만 유지한 뒤, 애플리케이션 시작 시점(예: main.py의
startup/lifespan 이벤트 핸들러)에서 validate_required_settings()를 호출하여 fail-fast 동작은
유지하되 import-time side effect를 제거하세요.
…sh language consistency, non-intrusive fail-fast validation)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/app/core/config.py (1)
26-32: 이전 리뷰 피드백을 잘 반영한 개선입니다.import 시점 예외를 제거하고 명시적으로 호출 가능한 함수로 분리하여, 테스트 수집 및 도구 실행 시 발생하던 문제를 해결했습니다.
main.py의 lifespan에서 호출되어 fail-fast 동작도 유지됩니다.타입 힌트 추가를 권장합니다:
♻️ 제안
-def validate_required_settings(): +def validate_required_settings() -> None:,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/config.py` around lines 26 - 32, Add an explicit return type hint to the validate_required_settings function (e.g., -> None) to improve type clarity; ensure the function signature is updated (validate_required_settings) and keep the body unchanged (checks against settings.SUPABASE_URL and settings.SUPABASE_SERVICE_KEY), and if your project uses typing for runtime settings objects consider annotating the settings variable or its attributes where defined so callers and type checkers can infer the expected types.
🤖 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/core/config.py`:
- Around line 26-32: Add an explicit return type hint to the
validate_required_settings function (e.g., -> None) to improve type clarity;
ensure the function signature is updated (validate_required_settings) and keep
the body unchanged (checks against settings.SUPABASE_URL and
settings.SUPABASE_SERVICE_KEY), and if your project uses typing for runtime
settings objects consider annotating the settings variable or its attributes
where defined so callers and type checkers can infer the expected types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e15101a5-3908-4277-9ffe-ed2a275b73fb
📒 Files selected for processing (3)
analysis_results.mdbackend/app/core/config.pybackend/app/main.py
✅ Files skipped from review due to trivial changes (1)
- analysis_results.md
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/main.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/app/core/config.py (1)
26-32: 이전 리뷰 의견이 잘 반영되었습니다.모듈 import 시점의 fail-fast 검증을
validate_required_settings()함수로 분리하여 테스트 및 도구 실행 시 import 단계에서의 실패를 방지했습니다. 이 함수를 FastAPI lifespan에서 호출하는 방식은 올바른 접근입니다.다만,
backend/scripts/ingest_data.py와 같이 FastAPI lifespan을 거치지 않고 직접config.py를 import하는 standalone 스크립트에서는 fail-fast 동작이 사라집니다. 필요시 해당 스크립트에서 명시적으로validate_required_settings()를 호출하는 것을 고려해 주세요.# ingest_data.py 상단에 추가 예시 from app.core.config import settings, validate_required_settings validate_required_settings() # 스크립트 시작 시 fail-fast 검증🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/config.py` around lines 26 - 32, validate_required_settings() was moved out of module import to avoid import-time failure, but standalone scripts that import config.py (e.g., backend/scripts/ingest_data.py) will no longer trigger fail-fast validation; update those scripts to explicitly call validate_required_settings() at startup after importing settings and validate_required_settings (e.g., in ingest_data.py import from app.core.config the settings and validate_required_settings and invoke validate_required_settings() before proceeding) so the script fails fast when SUPABASE_URL or SUPABASE_SERVICE_KEY are missing.
🤖 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/core/config.py`:
- Around line 26-32: validate_required_settings() was moved out of module import
to avoid import-time failure, but standalone scripts that import config.py
(e.g., backend/scripts/ingest_data.py) will no longer trigger fail-fast
validation; update those scripts to explicitly call validate_required_settings()
at startup after importing settings and validate_required_settings (e.g., in
ingest_data.py import from app.core.config the settings and
validate_required_settings and invoke validate_required_settings() before
proceeding) so the script fails fast when SUPABASE_URL or SUPABASE_SERVICE_KEY
are missing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3dc3ec01-684f-4b29-8b7e-a4e7a666f834
📒 Files selected for processing (1)
backend/app/core/config.py
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
릴리스 노트
보안 개선
배포 설정
문서