[Refactor] - 통합 테스트 및 UI 개선#54
Merged
Merged
Conversation
mapApiReportToDiagnosisResult를 analysis/_lib/mapApiReport.ts로 추출하여 history/[id] 페이지에서 재사용 가능하게 함. 기존 인라인 style 숫자 3개 표시 → DiagnosisDetailView 컴포넌트로 교체.
… 초기화 타이밍 수정 useEffect는 브라우저 페인트 이후에 실행되어 첫 렌더 시 user가 null로 유지되는 문제가 있었음. useLayoutEffect로 교체하여 페인트 전에 서버 유저를 auth 스토어에 동기화, 견적서 진단 '분석 시작' 버튼 비활성화 버그 수정.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
changed the title
|
Warning Review limit reached
More reviews will be available in 37 minutes and 8 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough로딩 화면 중복 제거를 통해 코드 응집성을 높이고, 견적 플로우를 5→4단계로 단순화하며, History를 서버 렌더링으로 전환하고, API에 userId를 추가하며, UserProvider로 인증 아키텍처를 개선합니다. 불필요한 UI(PDF 버튼, 화살표)를 제거하고 인사이트 기반 리포팅으로 변경합니다. ChangesLoadingScreen 컴포넌트 통합
Estimate 견적 생성 플로우 단순화 (5단계→4단계)
History 페이지 및 컴포넌트 구조 개선
API 인증 강화 및 UserProvider 도입
UI 요소 정리 및 스타일 정규화
Sequence Diagram(s)sequenceDiagram
participant App as Estimate Flow
participant Store as estimateStore
participant API as analyzeRisk API
participant View as EstimateResult
App->>Store: submitAnalysis(formData)
Store->>API: analyzeRisk(formData, userId, signal)
API->>API: 헤더: x-user-id
alt API 성공
API-->>Store: RiskDetectAnalyzeResponse
Store->>Store: mapApiReportToDiagnosisResult
Store->>View: result (DiagnosisResult)
View->>View: buildInsights(counts)
View-->>App: 종합 인사이트 렌더링
else API 오류 (401 외)
API-->>Store: 즉시 error throw
Store->>Store: err.body.detail 우선
Store-->>App: errorMessage
end
sequenceDiagram
participant Browser
participant Server as getServerMe/getServerEstimates
participant Client as HistoryPage (async)
participant Component as HistoryContent
Browser->>Client: /history 접근
Client->>Server: getServerMe()
Server-->>Client: user
Client->>Server: Promise.all([getServerEstimates, getServerRiskDetections])
Server-->>Client: estimates[], risks[]
Client->>Client: buildHistoryRows(estimates, risks)
Client->>Client: 정렬 (createdAt DESC)
Client->>Component: rows={HistoryRow[]}
Component-->>Browser: HistoryContent 렌더링
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
🔍 CI 검증 결과
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/app/(main)/estimate/page.tsx (1)
35-58:⚠️ Potential issue | 🟠 Major | ⚡ Quick win취소 후에도 비동기 결과가 store에 반영되는 경쟁 상태를 막아주세요.
Line 79에서 취소로 이전 단계 이동해도, 이미 시작된
fetchEstimate가 완료되면setEstimateResult/setGeneratingState가 실행됩니다. 취소 이후 stale 결과가 저장되어 다음 진입 시 재생성 로직이 건너뛰어질 수 있습니다.수정 예시
useEffect(() => { + let cancelled = false; if (currentStep !== 4 || estimateResult !== null || isGenerating || generateError !== null) { return; } @@ const fetchEstimate = async () => { setGeneratingState(true, null); try { @@ - setEstimateResult(result); - setGeneratingState(false, null); + if (cancelled) return; + setEstimateResult(result); + setGeneratingState(false, null); } catch { - setGeneratingState(false, '가견적 생성에 실패했습니다. 다시 시도해 주세요.'); + if (cancelled) return; + setGeneratingState(false, '가견적 생성에 실패했습니다. 다시 시도해 주세요.'); } }; fetchEstimate(); + return () => { + cancelled = true; + }; }, [Also applies to: 79-82, 100-103
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/`(main)/estimate/page.tsx around lines 35 - 58, The async fetchEstimate can complete after the user navigates away and then write stale results into the store (via setEstimateResult / setGeneratingState); prevent this by tracking cancellation for that specific invocation and skipping state updates when cancelled: inside the useEffect that declares fetchEstimate create a local cancellation token (e.g., an AbortController or a boolean isCancelled flag unique to this effect), pass it into the async flow (generateEstimate promise) and, before calling setEstimateResult or setGeneratingState in both success and catch branches, check the cancellation token and return early if cancelled; also ensure you set the token to cancelled in the cleanup function of the same useEffect so any in-flight fetchEstimate won’t mutate state after the user changes currentStep.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/api/client.ts`:
- Around line 59-62: 현재 beforeRetry 핸들러에서 status가 undefined인 경우에도 refresh 경로로
빠지므로 네트워크/타임아웃 오류에 대해 refresh를 막아야 합니다; apps/web/src/api/client.ts의 beforeRetry
블록에서 현재의 if (status !== undefined && status !== 401) { throw error; } 를 변경해 상태
코드가 401일 때만 refresh를 허용하도록 바꾸세요 (예: if (status === undefined || status !== 401)
throw error; 또는 더 간단히 if (status !== 401) throw error;), 이렇게 하면 status가
undefined(네트워크/타임아웃)일 때 triggerRefresh()가 호출되지 않습니다.
In `@apps/web/src/app/`(main)/_components/LoadingScreen/LoadingScreen.css.ts:
- Around line 127-178: progressWrap and tipCard use fixed pixel widths causing
horizontal overflow on narrow viewports; change them to be responsive by
replacing the fixed width with width: '100%' and add maxWidth: '320px' for
progressWrap and maxWidth: '480px' for tipCard (and set boxSizing: 'border-box'
if not already) so they shrink to their parent on small screens while keeping
the intended max size on larger screens; update the style objects named
progressWrap and tipCard accordingly.
In `@apps/web/src/app/`(main)/_components/LoadingScreen/LoadingScreen.tsx:
- Around line 82-149: The loading UI is only visual—add accessibility semantics:
make the message region (where messages[msgIdx] or warningMessage render) an
ARIA live region (e.g., aria-live="polite" and aria-atomic="true") so screen
readers announce changes and keep using showWarning, warningMessage, msgIdx,
messages and msgExiting as the content source; mark the progress bar element
(styles.progressFill container or its parent progressWrap) with
role="progressbar" and provide aria-valuenow={progress.value}, aria-valuemin="0"
and aria-valuemax="100" (using progress.value from the progress object) and keep
the visual transition; hide purely decorative elements (the mascot SVG,
mascotShadow, and each working dot/ DOT_DELAYS mapping) from assistive tech via
aria-hidden="true" or role="presentation".
In `@apps/web/src/app/`(main)/analysis/_lib/mapApiReport.ts:
- Around line 4-5: The inferItemStatus function currently only reclassifies
items as '누락' when item.status === '불분명' and item.title.includes('누락'); update
inferItemStatus to also treat items with status '불분명' and an empty or
whitespace-only title as '누락' (e.g., trim the title and check for zero length)
so that ApiProcessItem entries with blank titles are counted/displayed as
missing.
In `@apps/web/src/app/`(main)/analysis/_store/analysisStore.test.ts:
- Around line 35-40: The test adds items with statuses including '누락', '불분명',
and '중복' but only asserts missingCount; update the same test to assert the other
derived counts as well: verify missingCount increments for status '누락',
riskCount (or whatever mapping is used) increments for '불분명', and
insufficientCount increments for '중복' using the same items array in the test;
locate the assertions around missingCount, riskCount, and insufficientCount in
the test file and add expectations for all three to prevent regression when the
status->count mapping changes.
In `@apps/web/src/app/`(main)/analysis/_store/analysisStore.ts:
- Around line 115-117: The code treats an empty string from (err as { body?: {
detail?: string } }).body?.detail as a valid message, causing a blank UI; update
the logic that builds apiDetail/errorMessage (referencing apiDetail,
errorMessage, and the set call) to treat empty or whitespace-only strings as
missing by trimming and checking length (>0) before using it, and fall back to
the default '분석 중 오류가 발생했습니다.' when the trimmed detail is empty or undefined.
In
`@apps/web/src/app/`(main)/estimate/_components/Step2ProcessSelection/Step2ProcessSelection.tsx:
- Around line 93-107: When wallpaperScope === '부분 도배' ensure the user cannot
proceed with an empty partialRooms: add a validation in the
Step2ProcessSelection component that checks data.wallpaperScope and
(data.partialRooms ?? []).length, show a validation error and/or disable the
next/submit action until at least one room is selected (update the UI around the
SelectButton group handling partialRooms); additionally, update mapToApiPayload
to either omit partialRooms or throw/return a validation error when
wallpaperScope is '부분 도배' but partialRooms is empty so invalid payloads are
never sent. Apply the same fix to the other identical block referenced (lines
~259-266).
In `@apps/web/src/app/`(main)/history/page.tsx:
- Around line 35-38: Replace direct access to summary.chips when building the
risk counts with the same aggregation used in the detail page: call or reuse
mapApiReportToDiagnosisResult on the report (e.g.,
mapApiReportToDiagnosisResult(item.result.report)) and pull the missing/unclear
counts from that mapped result instead of item.result.report.summary.chips
(update the risk object construction where missing/unclear are set).
---
Outside diff comments:
In `@apps/web/src/app/`(main)/estimate/page.tsx:
- Around line 35-58: The async fetchEstimate can complete after the user
navigates away and then write stale results into the store (via
setEstimateResult / setGeneratingState); prevent this by tracking cancellation
for that specific invocation and skipping state updates when cancelled: inside
the useEffect that declares fetchEstimate create a local cancellation token
(e.g., an AbortController or a boolean isCancelled flag unique to this effect),
pass it into the async flow (generateEstimate promise) and, before calling
setEstimateResult or setGeneratingState in both success and catch branches,
check the cancellation token and return early if cancelled; also ensure you set
the token to cancelled in the cleanup function of the same useEffect so any
in-flight fetchEstimate won’t mutate state after the user changes currentStep.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f22652ab-114f-46ad-881e-90054325af7f
📒 Files selected for processing (46)
apps/web/src/api/analyze/api.tsapps/web/src/api/client.tsapps/web/src/app/(main)/_components/AuthStoreInitializer/AuthStoreInitializer.tsxapps/web/src/app/(main)/_components/LoadingScreen/LoadingScreen.css.tsapps/web/src/app/(main)/_components/LoadingScreen/LoadingScreen.tsxapps/web/src/app/(main)/_components/LoadingScreen/index.tsapps/web/src/app/(main)/analysis/_components/AnalysisLoadingScreen/AnalysisLoadingScreen.css.tsapps/web/src/app/(main)/analysis/_components/AnalysisLoadingScreen/AnalysisLoadingScreen.tsxapps/web/src/app/(main)/analysis/_components/DiagnosisReport/DiagnosisReport.tsxapps/web/src/app/(main)/analysis/_lib/mapApiReport.tsapps/web/src/app/(main)/analysis/_store/analysisStore.test.tsapps/web/src/app/(main)/analysis/_store/analysisStore.tsapps/web/src/app/(main)/analysis/page.css.tsapps/web/src/app/(main)/analysis/page.tsxapps/web/src/app/(main)/estimate/_components/EstimateLoadingScreen/EstimateLoadingScreen.css.tsapps/web/src/app/(main)/estimate/_components/EstimateLoadingScreen/EstimateLoadingScreen.tsxapps/web/src/app/(main)/estimate/_components/EstimateResultView/EstimateResultView.tsxapps/web/src/app/(main)/estimate/_components/ProcessCheckbox/ProcessCheckbox.css.tsapps/web/src/app/(main)/estimate/_components/ProcessCheckbox/ProcessCheckbox.tsxapps/web/src/app/(main)/estimate/_components/Step2ProcessSelection/Step2ProcessSelection.css.tsapps/web/src/app/(main)/estimate/_components/Step2ProcessSelection/Step2ProcessSelection.tsxapps/web/src/app/(main)/estimate/_components/Step4ProcessDetail/Step4ProcessDetail.css.tsapps/web/src/app/(main)/estimate/_components/Step4ProcessDetail/Step4ProcessDetail.tsxapps/web/src/app/(main)/estimate/_components/Step5EstimateReview/Step5EstimateReview.css.tsapps/web/src/app/(main)/estimate/_components/Step5EstimateReview/Step5EstimateReview.tsxapps/web/src/app/(main)/estimate/_components/StepIndicator/StepIndicator.tsxapps/web/src/app/(main)/estimate/_store/estimateStore.test.tsapps/web/src/app/(main)/estimate/_store/estimateStore.tsapps/web/src/app/(main)/estimate/_types/meta.tsapps/web/src/app/(main)/estimate/page.tsxapps/web/src/app/(main)/history/[id]/page.tsxapps/web/src/app/(main)/history/_components/DiagnosisDetailView/DiagnosisDetailView.tsxapps/web/src/app/(main)/history/_components/HistoryContent/HistoryContent.tsxapps/web/src/app/(main)/history/_components/HistoryTable/HistoryTable.css.tsapps/web/src/app/(main)/history/_components/HistoryTable/HistoryTable.tsxapps/web/src/app/(main)/history/_lib/historyFilter.test.tsapps/web/src/app/(main)/history/_mocks/history-detail.mock.tsapps/web/src/app/(main)/history/_mocks/history.mock.tsapps/web/src/app/(main)/history/_types/history.types.tsapps/web/src/app/(main)/history/page.tsxapps/web/src/app/(main)/home/_components/HistoryTable/HistoryTable.css.tsapps/web/src/app/(main)/home/_components/HistoryTable/HistoryTable.tsxapps/web/src/app/(main)/home/_components/SummaryCard/SummaryCard.css.tsapps/web/src/app/(main)/home/_components/SummaryCard/SummaryCard.tsxapps/web/src/app/(main)/home/page.tsxapps/web/src/app/(main)/layout.tsx
💤 Files with no reviewable changes (9)
- apps/web/src/app/(main)/history/_components/DiagnosisDetailView/DiagnosisDetailView.tsx
- apps/web/src/app/(main)/history/_mocks/history-detail.mock.ts
- apps/web/src/app/(main)/analysis/_components/DiagnosisReport/DiagnosisReport.tsx
- apps/web/src/app/(main)/estimate/_components/EstimateLoadingScreen/EstimateLoadingScreen.css.ts
- apps/web/src/app/(main)/analysis/_components/AnalysisLoadingScreen/AnalysisLoadingScreen.css.ts
- apps/web/src/app/(main)/estimate/_components/Step4ProcessDetail/Step4ProcessDetail.tsx
- apps/web/src/app/(main)/history/_mocks/history.mock.ts
- apps/web/src/app/(main)/estimate/_components/Step4ProcessDetail/Step4ProcessDetail.css.ts
- apps/web/src/app/(main)/home/_components/HistoryTable/HistoryTable.css.ts
Comment on lines
+35
to
+40
| items: [ | ||
| { title: '일반 철거', description: '180만원', guide: '', status: '정상' as const }, | ||
| { title: '철거 폐기물 처리비', description: '누락된 항목', guide: '', status: '누락' as const }, | ||
| { title: '기타 비용', description: '불분명 항목', guide: '', status: '불분명' as const }, | ||
| { title: '인건비', description: '중복 항목', guide: '', status: '중복' as const }, | ||
| ], |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
확장한 상태 케이스에 대한 단정문을 같이 추가해 주세요.
목 데이터는 불분명/중복까지 늘렸는데, 핵심 검증은 아직 missingCount 위주라 매핑 회귀를 놓칠 수 있습니다. riskCount, insufficientCount도 동일 테스트에서 확인하는 게 좋습니다.
추가 단정 예시
expect(state.diagnosisResult?.vendorLabel).toBe('A업체');
expect(state.diagnosisResult?.missingCount).toBe(1);
+ expect(state.diagnosisResult?.riskCount).toBe(1);
+ expect(state.diagnosisResult?.insufficientCount).toBe(1);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/`(main)/analysis/_store/analysisStore.test.ts around lines
35 - 40, The test adds items with statuses including '누락', '불분명', and '중복' but
only asserts missingCount; update the same test to assert the other derived
counts as well: verify missingCount increments for status '누락', riskCount (or
whatever mapping is used) increments for '불분명', and insufficientCount increments
for '중복' using the same items array in the test; locate the assertions around
missingCount, riskCount, and insufficientCount in the test file and add
expectations for all three to prevent regression when the status->count mapping
changes.
Comment on lines
+115
to
+117
| const apiDetail = (err as { body?: { detail?: string } }).body?.detail; | ||
| const errorMessage = apiDetail ?? '분석 중 오류가 발생했습니다.'; | ||
| set({ error: errorMessage, loading: false }); |
There was a problem hiding this comment.
detail이 빈 문자열일 때 에러 문구가 비어 보일 수 있습니다.
현재 apiDetail ?? ...는 ''를 유효값으로 처리해서, 서버가 빈 detail을 보내면 UI에 빈 오류 문구가 표시됩니다. 공백/빈 문자열은 기본 메시지로 폴백하는 쪽이 안전합니다.
수정 예시
- const apiDetail = (err as { body?: { detail?: string } }).body?.detail;
- const errorMessage = apiDetail ?? '분석 중 오류가 발생했습니다.';
+ const apiDetail = (err as { body?: { detail?: string } }).body?.detail;
+ const errorMessage =
+ typeof apiDetail === 'string' && apiDetail.trim().length > 0
+ ? apiDetail
+ : '분석 중 오류가 발생했습니다.';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/`(main)/analysis/_store/analysisStore.ts around lines 115 -
117, The code treats an empty string from (err as { body?: { detail?: string }
}).body?.detail as a valid message, causing a blank UI; update the logic that
builds apiDetail/errorMessage (referencing apiDetail, errorMessage, and the set
call) to treat empty or whitespace-only strings as missing by trimming and
checking length (>0) before using it, and fall back to the default '분석 중 오류가
발생했습니다.' when the trimmed detail is empty or undefined.
KyeongJooni
reviewed
May 27, 2026
KyeongJooni
left a comment
KyeongJooni
left a comment
Contributor
There was a problem hiding this comment.
수고하셨습니다!! 코멘트 확인해주세요 :)
| }, [user, setUser]); | ||
|
|
||
| return null; | ||
| }; |
Contributor
There was a problem hiding this comment.
현재 사용자 인증을 서버에서 직접 하고 있는데 zustand로 인증을 관리하는 이유가 있나요?
Collaborator
Author
There was a problem hiding this comment.
UploadPanel, Step5EstimateReview에서 API 호출 시 user.id가 필요한데,
서버 전용 함수를 직접 호출할 수 없어서 AuthStoreInitializer가 user를 Zustand에 주입해주는 구조입니다.
Contributor
There was a problem hiding this comment.
인증한정으로 user.id를 사용하는 거면 React Context (UserProvider) 로도 동일하게 server → client로 user 데이터를 흘려보낼 수 있고, 이쪽이 더 단순할 것 같습니다!!
Collaborator
Author
There was a problem hiding this comment.
아하 굳이 zudstand을 사용할 필요는 없었던 것 같네요! 반영하겠습니다🫡
Comment on lines
+86
to
+122
| <svg className={styles.mascotImg} viewBox="0 0 318 225" fill="none" xmlns="http://www.w3.org/2000/svg"> | ||
| <path | ||
| d="M223.42 0C275.175 0 317.132 41.8155 317.132 93.3975C317.132 93.4476 317.131 93.4977 317.131 93.5479V179.104C316.852 186.528 317.902 197.168 310.961 207.93C304.019 218.692 291.293 222.535 279.723 206.008C277.366 202.641 271.45 200.284 267.768 206.008C258.127 220.998 240.388 221.766 229.59 206.008C226.864 202.031 219.692 201.636 216.863 206.008C207.413 220.613 190.254 221.766 178.684 206.008C175.985 201.012 169.732 200.628 166.344 206.008C158.118 219.076 145.519 220.614 136.649 209.083C132.065 202.396 130.395 197.783 129.708 187.276V93.3936H129.709C129.711 41.8134 171.666 9.71069e-05 223.42 0Z" | ||
| fill="#8455DF" | ||
| /> | ||
| <path | ||
| d="M223.813 28.9412C261.903 28.9412 292.729 57.9991 292.729 93.781C292.729 129.563 261.903 158.621 223.813 158.621C185.723 158.621 154.897 129.563 154.897 93.781C154.897 57.9991 185.723 28.9412 223.813 28.9412Z" | ||
| fill="white" | ||
| stroke="#8455DF" | ||
| /> | ||
| <rect | ||
| x="188.056" | ||
| y="95.0508" | ||
| width="16.7396" | ||
| height="30.5168" | ||
| rx="8.3698" | ||
| fill="#8455DF" | ||
| stroke="#8455DF" | ||
| /> | ||
| <rect | ||
| x="238.188" | ||
| y="95.0508" | ||
| width="16.7396" | ||
| height="30.5168" | ||
| rx="8.3698" | ||
| fill="#8455DF" | ||
| stroke="#8455DF" | ||
| /> | ||
| <path | ||
| d="M10.5085 127.034C12.4797 119.917 20.1856 114.827 24.049 111.845C36.6964 102.603 64.7902 82.015 72.2542 76.5606C79.7183 71.1061 84.3583 74.3151 85.8508 76.5606C86.9066 77.7862 90.494 82.8818 98.4228 94.1442L99.2493 95.3182C109.884 110.424 97.1264 112.066 85.8508 114.394C79.5814 115.689 80.3527 121.838 83.8235 127.219L114.457 172.128C116.959 176.179 120.7 186.762 109.662 194.097C99.2495 201.015 89.3928 193.328 85.1405 186.88C79.6263 178.518 67.1428 159.426 61.3218 149.953C54.201 138.366 52.6641 136.564 44.1022 141.44C33.304 147.59 35.6751 160.593 29.1858 161.522C22.6965 162.451 20.3332 159.889 14.3422 150.446C8.3513 141.002 8.61658 133.866 10.5085 127.034Z" | ||
| fill="#8455DF" | ||
| stroke="#8455DF" | ||
| /> | ||
| </svg> | ||
| </div> | ||
| <div className={styles.mascotShadow} /> | ||
| </div> |
Contributor
There was a problem hiding this comment.
인라인 svg 말고 svg 파일을 만들어서 사용하는게 어떨까요?
Contributor
There was a problem hiding this comment.
메시지 회전과 팁 회전이 동일 패턴 두 번 반복 되고 있는 것 같은데, useRotatingIndex(count, interval, exit) 커스텀 훅으로 추출하면 좋을 것 같아요!
LoadingScreen/
LoadingScreen.tsx ← 얇은 조립
LoadingScreen.css.ts
_components/
Mascot.tsx ← SVG 분리
WorkingDots.tsx
RotatingMessage.tsx ← message + warning swap
ProgressBar.tsx
TipCard.tsx
LoadingFooter.tsx
_hooks/
useRotatingIndex.ts ← message/tip 공통 로직
useProgressSteps.ts
useDelayedFlag.ts ← warning 노출
이런식으로 분리해서 로딩스크린 작성하면 좋을 것 같습니다! 😄
Comment on lines
+45
to
+79
| useEffect(() => { | ||
| const timers = progressSteps.map(({ delay, value, duration, easing }) => | ||
| setTimeout(() => setProgress({ value, duration, easing }), delay) | ||
| ); | ||
| return () => timers.forEach(clearTimeout); | ||
| }, [progressSteps]); | ||
|
|
||
| useEffect(() => { | ||
| const id = setInterval(() => { | ||
| setMsgExiting(true); | ||
| const tid = setTimeout(() => { | ||
| setMsgIdx((i) => (i + 1) % messages.length); | ||
| setMsgExiting(false); | ||
| }, 270); | ||
| return () => clearTimeout(tid); | ||
| }, 4000); | ||
| return () => clearInterval(id); | ||
| }, [messages.length]); | ||
|
|
||
| useEffect(() => { | ||
| const id = setInterval(() => { | ||
| setTipExiting(true); | ||
| const tid = setTimeout(() => { | ||
| setTipIdx((i) => (i + 1) % tips.length); | ||
| setTipExiting(false); | ||
| }, 330); | ||
| return () => clearTimeout(tid); | ||
| }, 8000); | ||
| return () => clearInterval(id); | ||
| }, [tips.length]); | ||
|
|
||
| useEffect(() => { | ||
| const t = setTimeout(() => setShowWarning(true), warningDelayMs); | ||
| return () => clearTimeout(t); | ||
| }, [warningDelayMs]); |
Contributor
There was a problem hiding this comment.
SRP 원칙을 지켜서 UI 컴포넌트안에 있는 훅 핸들러들을 분리하는게 좋을 것 같습니다!!
🔍 CI 검증 결과
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/web/src/api/client.ts (1)
59-61:⚠️ Potential issue | 🟠 Major | ⚡ Quick winbeforeRetry에서 401 상태코드는
error.status가 아니라HTTPError.response.status로 읽어야 합니다.
apps/web/src/api/client.ts의beforeRetry에서const status = (error as { status?: number }).status;로 상태코드를 읽고 있어401일 때도status가undefined로 떨어져 곧바로throw될 수 있습니다.ky의 HTTP 오류는HTTPError로 전달되며 상태코드는error.response.status에 있습니다.수정 예시
-import ky, { type KyInstance } from 'ky'; +import ky, { HTTPError, type KyInstance } from 'ky'; @@ - const status = (error as { status?: number }).status; + const status = + error instanceof HTTPError ? error.response.status : (error as { status?: number }).status; if (status !== 401) { throw error; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/api/client.ts` around lines 59 - 61, In beforeRetry change how the HTTP status is read: instead of reading (error as { status?: number }).status, access the ky HTTPError response status (e.g. (error as HTTPError).response.status or error.response?.status) so 401 is detected correctly; update beforeRetry to import/recognize HTTPError (or use a safe optional chain) and only rethrow when status !== 401, otherwise handle retry logic as intended.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@apps/web/src/api/client.ts`:
- Around line 59-61: In beforeRetry change how the HTTP status is read: instead
of reading (error as { status?: number }).status, access the ky HTTPError
response status (e.g. (error as HTTPError).response.status or
error.response?.status) so 401 is detected correctly; update beforeRetry to
import/recognize HTTPError (or use a safe optional chain) and only rethrow when
status !== 401, otherwise handle retry logic as intended.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f9233733-9320-4bfd-bd3b-8b62909043fb
📒 Files selected for processing (6)
apps/web/src/api/client.tsapps/web/src/app/(main)/_components/LoadingScreen/LoadingScreen.css.tsapps/web/src/app/(main)/_components/LoadingScreen/LoadingScreen.tsxapps/web/src/app/(main)/analysis/_lib/mapApiReport.tsapps/web/src/app/(main)/estimate/_components/Step2ProcessSelection/Step2ProcessSelection.tsxapps/web/src/app/(main)/history/page.tsx
🔍 CI 검증 결과
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/app/(main)/_components/LoadingScreen/LoadingScreen.tsx (1)
24-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick win빈
messages/tips입력 방어 로직이 필요합니다.Line 36, Line 38에서 배열을 그대로 전달하고 있어 빈 배열 입력 시 메시지/팁 렌더링이 깨질 수 있습니다(하위 컴포넌트 인덱싱/진행률 계산 영향). 공용 컴포넌트 진입점에서 기본값으로 정규화해 주세요.
수정 예시
export const LoadingScreen = ({ messages, tips, warningMessage, warningDelayMs, progressSteps, footerText, onCancel, -}: Props) => ( - <div className={styles.container}> - <Mascot /> - <WorkingDots /> - <RotatingMessage messages={messages} warningMessage={warningMessage} warningDelayMs={warningDelayMs} /> - <ProgressBar steps={progressSteps} /> - <TipCard tips={tips} /> - <LoadingFooter footerText={footerText} onCancel={onCancel} /> - </div> -); +}: Props) => { + const safeMessages = messages.length > 0 ? messages : ['처리 중입니다...']; + const safeTips = tips.length > 0 ? tips : ['잠시만 기다려 주세요.']; + + return ( + <div className={styles.container}> + <Mascot /> + <WorkingDots /> + <RotatingMessage messages={safeMessages} warningMessage={warningMessage} warningDelayMs={warningDelayMs} /> + <ProgressBar steps={progressSteps} /> + <TipCard tips={safeTips} /> + <LoadingFooter footerText={footerText} onCancel={onCancel} /> + </div> + ); +};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/`(main)/_components/LoadingScreen/LoadingScreen.tsx around lines 24 - 40, Normalize empty or undefined messages and tips at the LoadingScreen entry: in the LoadingScreen component ensure the props messages and tips are replaced with safe defaults (e.g., messages ?? [] or messages?.length ? messages : [default] and likewise for tips) before passing them to RotatingMessage and TipCard so downstream indexing/progress logic never receives an empty/undefined array; update the LoadingScreen parameter handling (Props destructuring) to compute safeMessages and safeTips and pass those to RotatingMessage and TipCard respectively.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/web/src/app/`(main)/_components/LoadingScreen/_components/ProgressBar.tsx:
- Line 18: The ProgressBar component currently hardcodes aria-label="분석 진행률",
which is incorrect for a shared component; update the ProgressBar (in
ProgressBar.tsx) to accept a prop (e.g., ariaLabel or label) with a sensible
default like "진행 상태" and replace the fixed aria-label with that prop so callers
(like the estimate flow) can pass a context-specific string; ensure the prop is
optional and typed in the component's props/interface and used where aria-label
is set.
In
`@apps/web/src/app/`(main)/_components/LoadingScreen/_hooks/useProgressSteps.ts:
- Line 32: The useEffect in useProgressSteps depends on the steps reference
(useEffect(..., [steps])) which assumes callers pass a stable array; add a short
comment in useProgressSteps stating the API expectation that callers (e.g.,
ProgressBar / LoadingScreen) must pass a stable/memoized steps array (such as
the PROGRESS_STEPS constant used by AnalysisLoadingScreen and
EstimateLoadingScreen) or wrap dynamic step arrays with useMemo to avoid timer
churn, and mention this expectation in the hook's docstring or adjacent comment
so future callers know to preserve reference stability.
In
`@apps/web/src/app/`(main)/_components/LoadingScreen/_hooks/useRotatingIndex.ts:
- Around line 29-39: The effect in useRotatingIndex creates setTimeout timers
(tid) inside the setInterval callback but never clears them because the return
inside the interval callback is ignored; fix by tracking timeout ids in a ref or
array (e.g., timeoutsRef) and clearing any pending timeout(s) when the effect
cleanup runs and before creating a new timeout; ensure you still
clearInterval(id) and clearTimeout for all stored tids (use clearTimeout on each
id) and keep using the existing symbols setInterval, setTimeout, id, tid,
exitDuration, interval, setExiting, setIndex, and count to locate and update the
logic.
---
Outside diff comments:
In `@apps/web/src/app/`(main)/_components/LoadingScreen/LoadingScreen.tsx:
- Around line 24-40: Normalize empty or undefined messages and tips at the
LoadingScreen entry: in the LoadingScreen component ensure the props messages
and tips are replaced with safe defaults (e.g., messages ?? [] or
messages?.length ? messages : [default] and likewise for tips) before passing
them to RotatingMessage and TipCard so downstream indexing/progress logic never
receives an empty/undefined array; update the LoadingScreen parameter handling
(Props destructuring) to compute safeMessages and safeTips and pass those to
RotatingMessage and TipCard respectively.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 518a3e02-0549-4126-8ac6-3d0ebcaef02f
📒 Files selected for processing (19)
apps/web/src/app/(main)/_components/LoadingScreen/LoadingScreen.css.tsapps/web/src/app/(main)/_components/LoadingScreen/LoadingScreen.tsxapps/web/src/app/(main)/_components/LoadingScreen/_components/LoadingFooter.tsxapps/web/src/app/(main)/_components/LoadingScreen/_components/Mascot.tsxapps/web/src/app/(main)/_components/LoadingScreen/_components/ProgressBar.tsxapps/web/src/app/(main)/_components/LoadingScreen/_components/RotatingMessage.tsxapps/web/src/app/(main)/_components/LoadingScreen/_components/TipCard.tsxapps/web/src/app/(main)/_components/LoadingScreen/_components/WorkingDots.tsxapps/web/src/app/(main)/_components/LoadingScreen/_hooks/useDelayedFlag.tsapps/web/src/app/(main)/_components/LoadingScreen/_hooks/useProgressSteps.tsapps/web/src/app/(main)/_components/LoadingScreen/_hooks/useRotatingIndex.tsapps/web/src/app/(main)/_components/UserProvider/UserProvider.tsxapps/web/src/app/(main)/analysis/_components/DiagnosisReport/DiagnosisReport.css.tsapps/web/src/app/(main)/analysis/_components/DiagnosisReport/DiagnosisReport.tsxapps/web/src/app/(main)/analysis/_components/UploadPanel/UploadPanel.tsxapps/web/src/app/(main)/analysis/_lib/buildInsights.tsapps/web/src/app/(main)/estimate/_components/Step5EstimateReview/Step5EstimateReview.tsxapps/web/src/app/(main)/layout.tsxapps/web/src/store/authStore.ts
💤 Files with no reviewable changes (1)
- apps/web/src/store/authStore.ts
| aria-valuenow={value} | ||
| aria-valuemin={0} | ||
| aria-valuemax={100} | ||
| aria-label="분석 진행률" |
There was a problem hiding this comment.
공용 컴포넌트인데 aria-label이 분석 전용 문구로 고정되어 있습니다.
현재 라벨은 견적 플로우에서도 “분석 진행률”로 읽혀 문맥이 어긋납니다. 공용 문구(예: 진행 상태)로 바꾸거나 prop으로 주입해 주세요.
수정 예시
- aria-label="분석 진행률"
+ aria-label="진행 상태"📝 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
| aria-label="분석 진행률" | |
| aria-label="진행 상태" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/web/src/app/`(main)/_components/LoadingScreen/_components/ProgressBar.tsx
at line 18, The ProgressBar component currently hardcodes aria-label="분석 진행률",
which is incorrect for a shared component; update the ProgressBar (in
ProgressBar.tsx) to accept a prop (e.g., ariaLabel or label) with a sensible
default like "진행 상태" and replace the fixed aria-label with that prop so callers
(like the estimate flow) can pass a context-specific string; ensure the prop is
optional and typed in the component's props/interface and used where aria-label
is set.
| setTimeout(() => setProgress({ value, duration, easing }), delay) | ||
| ); | ||
| return () => timers.forEach(clearTimeout); | ||
| }, [steps]); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: useProgressSteps 호출부에서 steps 전달 방식 확인
rg -nP -A5 -B2 'useProgressSteps\(' --type=ts --type=tsxRepository: MaxedOut-Muneo/Muneo-Client
Length of output: 99
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) 훅 구현 확인
HOOK_FILE="apps/web/src/app/(main)/_components/LoadingScreen/_hooks/useProgressSteps.ts"
echo "=== HOOK FILE (head) ==="
wc -l "$HOOK_FILE"
sed -n '1,200p' "$HOOK_FILE" | nl -ba
# 2) useProgressSteps 호출부 찾기 (tsx filetype은 rg 환경마다 다를 수 있어 -g로 안전하게)
echo "=== CALL SITES (useProgressSteps) ==="
rg -n "useProgressSteps\s*\(" -S apps/web/src --glob='*.ts' --glob='*.tsx' --glob='*.js' --glob='*.jsx'
# 3) 호출부의 steps 전달 방식(가능한 한 가까운 라인까지) 살펴보기
echo "=== CONTEXT AROUND CALL SITES ==="
# useProgressSteps( 근처에 steps를 넘기는 형태를 찾기 위해 steps= 혹은 steps 인자를 대략적으로 필터
rg -n "useProgressSteps\s*\(" -S apps/web/src --glob='*.ts' --glob='*.tsx' --glob='*.js' --glob='*.jsx' -o \
| head -n 50 >/tmp/calls.txt || true
# 각 호출이 있는 파일/라인을 기준으로 주변 문맥 출력
while IFS=: read -r file line; do
echo "---- $file:$line ----"
sed -n "$((line-10)),$((line+20))p" "$file" | nl -ba
done < <(cut -d: -f1-2 /tmp/calls.txt 2>/dev/null | sort -u) || trueRepository: MaxedOut-Muneo/Muneo-Client
Length of output: 237
🏁 Script executed:
#!/bin/bash
set -euo pipefail
HOOK_FILE="apps/web/src/app/(main)/_components/LoadingScreen/_hooks/useProgressSteps.ts"
echo "=== HOOK FILE ==="
wc -l "$HOOK_FILE"
echo "=== HOOK CONTENT ==="
sed -n '1,120p' "$HOOK_FILE" | cat -n
echo "=== CALL SITES (useProgressSteps) ==="
rg -n "useProgressSteps\s*\(" apps/web/src --glob='*.ts' --glob='*.tsx' --glob='*.js' --glob='*.jsx' -S || trueRepository: MaxedOut-Muneo/Muneo-Client
Length of output: 1448
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/web/src/app/(main)/_components/LoadingScreen/_components/ProgressBar.tsx"
echo "=== FILE ==="
wc -l "$FILE"
echo "=== CONTENT ==="
sed -n '1,200p' "$FILE" | cat -nRepository: MaxedOut-Muneo/Muneo-Client
Length of output: 1011
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# ProgressBar 호출부 찾기
rg -n "ProgressBar\s*[^=]*\s*steps=" apps/web/src --glob='*.ts' --glob='*.tsx' --glob='*.js' --glob='*.jsx' -S || true
# 단순히 ProgressBar 사용 위치 전체 스캔(steps 인자 포함 여부 확인용)
rg -n "ProgressBar" apps/web/src --glob='*.ts' --glob='*.tsx' -S || trueRepository: MaxedOut-Muneo/Muneo-Client
Length of output: 555
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="apps/web/src/app/(main)/_components/LoadingScreen/LoadingScreen.tsx"
echo "=== FILE ==="
wc -l "$FILE"
echo "=== CONTENT ==="
sed -n '1,200p' "$FILE" | cat -nRepository: MaxedOut-Muneo/Muneo-Client
Length of output: 1617
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# LoadingScreen 호출부 찾기
rg -n "<LoadingScreen" apps/web/src --glob='*.ts' --glob='*.tsx' --glob='*.js' --glob='*.jsx' -S || true
# progressSteps 전달하는 형태만 더 좁혀서 확인
rg -n "progressSteps\s*=" apps/web/src --glob='*.ts' --glob='*.tsx' --glob='*.js' --glob='*.jsx' -S || trueRepository: MaxedOut-Muneo/Muneo-Client
Length of output: 567
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILES=(
"apps/web/src/app/(main)/analysis/_components/AnalysisLoadingScreen/AnalysisLoadingScreen.tsx"
"apps/web/src/app/(main)/estimate/_components/EstimateLoadingScreen/EstimateLoadingScreen.tsx"
)
for f in "${FILES[@]}"; do
echo "=== FILE: $f ==="
wc -l "$f"
sed -n '1,220p' "$f" | cat -n
echo
doneRepository: MaxedOut-Muneo/Muneo-Client
Length of output: 4012
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== LoadingScreen usages ==="
rg -n "<LoadingScreen\b" apps/web/src --glob='*.ts' --glob='*.tsx' --glob='*.js' --glob='*.jsx' -S || true
echo
echo "=== progressSteps prop assignments ==="
rg -n "progressSteps\s*=" apps/web/src --glob='*.ts' --glob='*.tsx' --glob='*.js' --glob='*.jsx' -S || trueRepository: MaxedOut-Muneo/Muneo-Client
Length of output: 636
useProgressSteps의 steps 의존성은 현재 안정적이라 타이머 재생성 이슈 없음
useProgressSteps에서 useEffect(..., [steps])로 steps 참조에 의존하는 구조입니다. 다만 현재 useProgressSteps 호출부는 ProgressBar이고, LoadingScreen에서 전달되는 progressSteps는 PROGRESS_STEPS 모듈 상수로만 제공되어(AnalysisLoadingScreen/EstimateLoadingScreen) 렌더마다 새 배열이 만들어지지 않습니다.
향후 다른 곳에서 steps를 동적으로 생성해 넘길 경우에만 의존성 변경으로 타이머 churn이 생길 수 있으니, API 기대사항(참조 안정성/메모이즈)을 주석이나 문서로 남기는 정도를 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/`(main)/_components/LoadingScreen/_hooks/useProgressSteps.ts
at line 32, The useEffect in useProgressSteps depends on the steps reference
(useEffect(..., [steps])) which assumes callers pass a stable array; add a short
comment in useProgressSteps stating the API expectation that callers (e.g.,
ProgressBar / LoadingScreen) must pass a stable/memoized steps array (such as
the PROGRESS_STEPS constant used by AnalysisLoadingScreen and
EstimateLoadingScreen) or wrap dynamic step arrays with useMemo to avoid timer
churn, and mention this expectation in the hook's docstring or adjacent comment
so future callers know to preserve reference stability.
🔍 CI 검증 결과
|
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
2 participants
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.
✨ 주요 변경사항
📝 작업 상세 내용
인증
API
견적서 진단
가견적서 생성
분석 이력
홈
✅ 체크리스트
Close #번호추가📸 스크린샷 (선택)
🔍 기타 참고사항
로딩 화면 팁 카드 개선 의도
서비스 유입 사용자는 인테리어 시공 지식이 부족한 경우가 많습니다. 분석 대기 시간(최대 1~2분)을 단순 대기가 아닌 유용한 정보를 전달하는 시간으로 활용하기 위해 팁 카드를 주요 UI 요소로 격상했습니다.
mapApiReport 카운트 로직 변경
기존에는 백엔드 summary.chips 값을 그대로 사용했으나, 실제 items를 순회해 집계하는 방식으로 변경했습니다. 이에 따라 analysisStore.test.ts의 목 데이터도 실제 items에 리스크 항목을 포함하도록 수정했습니다.
🔗 관련 이슈
Summary by CodeRabbit
릴리즈 노트
새로운 기능
개선 사항
UI/UX 개선