Frontend Refactor PR Report#261
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThe PR reorganizes frontend hooks into semantic subdirectories (blog/, career/), exports hook API types and pure helpers, refactors a monolithic form component into section-based composition, consolidates test setup patterns across multiple hooks, expands payload-builder test coverage, and improves backend auth and worker test robustness. ChangesFrontend Module Reorganization and Test Refactoring
Backend Test Robustness Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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.
🧹 Nitpick comments (2)
frontend/src/components/forms/sections/CareerQualificationsSection.tsx (1)
17-17: ⚡ Quick winImport React setter types explicitly to avoid namespace-dependent typing.
Using
React.Dispatchhere can be fragile depending on TS config. Prefer explicit type imports for stable module typing.Suggested patch
+import type { Dispatch, SetStateAction } from "react"; import { blankResumeQualification } from "../../../constants"; import type { CareerFormState } from "../../../payloadBuilders"; import type { ResumeQualification } from "../../../types"; import shared from "../../../styles/shared.module.css"; import { Skeleton } from "../../ui/Skeleton"; import { Combobox } from "../Combobox"; @@ - setForm: React.Dispatch<React.SetStateAction<CareerFormState>>; + setForm: Dispatch<SetStateAction<CareerFormState>>; };🤖 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 `@frontend/src/components/forms/sections/CareerQualificationsSection.tsx` at line 17, The prop type for setForm should use explicit React types instead of the namespace form; import the types with "import type { Dispatch, SetStateAction } from 'react';" and change the prop signature from "setForm: React.Dispatch<React.SetStateAction<CareerFormState>>;" to "setForm: Dispatch<SetStateAction<CareerFormState>>;" so the component (e.g., CareerQualificationsSection and its props interface) uses the stable, explicitly imported types.frontend/src/hooks/useDocumentForm.test.ts (1)
52-65: ⚡ Quick winConsider making the cache key configurable.
The
setuphelper hardcodes the cache key as"career"on line 60, but the actualcacheKeyis passed viaoverridesto the hook. This creates an implicit coupling that isn't enforced by types. If a test passes a differentcacheKeyinoverrides, the preset cache won't match, leading to confusing failures.♻️ Suggested fix to derive cache key from overrides
function setup( overrides: Partial<UseDocumentFormOptions<TestForm, TestPayload, TestResponse>> = {}, - storeOverrides: { presetCache?: { form: TestForm; documentId: string | null } } = {}, + storeOverrides: { + cacheKey?: string; + presetCache?: { form: TestForm; documentId: string | null }; + } = {}, ) { const store = createTestStore(); if (storeOverrides.presetCache) { + const cacheKey = storeOverrides.cacheKey ?? overrides.cacheKey ?? "career"; store.dispatch( setCache({ - key: "career", + key: cacheKey, form: storeOverrides.presetCache.form, documentId: storeOverrides.presetCache.documentId, }), ); }Then update the test call:
const { result } = setup( { cacheKey: "career" }, - { presetCache: { form: { title: "cached title" }, documentId: "doc-cached" } }, + { cacheKey: "career", presetCache: { form: { title: "cached title" }, documentId: "doc-cached" } }, );🤖 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 `@frontend/src/hooks/useDocumentForm.test.ts` around lines 52 - 65, The setup helper hardcodes the cache key "career" which can mismatch the cacheKey passed in overrides to UseDocumentFormOptions; update setup to derive the cache key from the overrides (e.g., read overrides.cacheKey or a default) before calling store.dispatch(setCache(...)) so the presetCache uses the same key the hook will use; locate the setup function and change the hardcoded "career" to use the override value (falling back to "career" if not provided) to ensure tests that pass a different cacheKey in overrides work correctly.
🤖 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.
Nitpick comments:
In `@frontend/src/components/forms/sections/CareerQualificationsSection.tsx`:
- Line 17: The prop type for setForm should use explicit React types instead of
the namespace form; import the types with "import type { Dispatch,
SetStateAction } from 'react';" and change the prop signature from "setForm:
React.Dispatch<React.SetStateAction<CareerFormState>>;" to "setForm:
Dispatch<SetStateAction<CareerFormState>>;" so the component (e.g.,
CareerQualificationsSection and its props interface) uses the stable, explicitly
imported types.
In `@frontend/src/hooks/useDocumentForm.test.ts`:
- Around line 52-65: The setup helper hardcodes the cache key "career" which can
mismatch the cacheKey passed in overrides to UseDocumentFormOptions; update
setup to derive the cache key from the overrides (e.g., read overrides.cacheKey
or a default) before calling store.dispatch(setCache(...)) so the presetCache
uses the same key the hook will use; locate the setup function and change the
hardcoded "career" to use the override value (falling back to "career" if not
provided) to ensure tests that pass a different cacheKey in overrides work
correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c870e7f7-9d93-4b02-92ae-e8e2d3cde2a8
📒 Files selected for processing (31)
backend/tests/auth/test_endpoints.pybackend/tests/auth/test_oauth_flow.pybackend/tests/test_worker/test_execute_task.pyfrontend/src/components/analysis/GitHubAnalysisPage.test.tsxfrontend/src/components/blog/BlogPage.tsxfrontend/src/components/blog/BlogPlatformList.tsxfrontend/src/components/career-analysis/CareerAnalysisPage.tsxfrontend/src/components/forms/CareerResumeForm.tsxfrontend/src/components/forms/ProjectModal.tsxfrontend/src/components/forms/sections/CareerBasicInfoSection.tsxfrontend/src/components/forms/sections/CareerExperienceSection.tsxfrontend/src/components/forms/sections/CareerQualificationsSection.tsxfrontend/src/components/forms/sections/CareerSelfPrSection.tsxfrontend/src/hooks/analysis/useAsyncAnalysisPage.test.tsfrontend/src/hooks/analysis/useAsyncAnalysisPage.tsfrontend/src/hooks/blog/useBlogAccountManager.test.tsfrontend/src/hooks/blog/useBlogAccountManager.tsfrontend/src/hooks/blog/useBlogSummaryPolling.tsfrontend/src/hooks/career/useCareerAnalysisPage.test.tsfrontend/src/hooks/career/useCareerAnalysisPage.tsfrontend/src/hooks/career/useCareerExperienceMutators.tsfrontend/src/hooks/career/usePhotoUpload.test.tsfrontend/src/hooks/career/usePhotoUpload.tsfrontend/src/hooks/career/useProjectModalForm.test.tsfrontend/src/hooks/career/useProjectModalForm.tsfrontend/src/hooks/career/useProjectModalState.test.tsfrontend/src/hooks/career/useProjectModalState.tsfrontend/src/hooks/useDocumentForm.test.tsfrontend/src/hooks/useDocumentForm.tsfrontend/src/hooks/useTaskPolling.test.tsfrontend/src/payloadBuilders.test.ts
This was referenced May 29, 2026
This was referenced Jun 6, 2026
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
useProjectModalForm.initProjectの 14 フィールド独立再定義をstructuredClone(blankCareerProject)に置換して SSoT 違反を解消。5 つのテストファイル (useDocumentForm/useTaskPolling/useAsyncAnalysisPage/useCareerAnalysisPage/GitHubAnalysisPage) に setup factory を導入して arrange コピペを集約。payloadBuilders.test.tsを 27 行 → 280 行に拡充し境界値テスト 26 件追加。useBlogAccountManagerの race condition guard をreduceActions純粋関数として切り出し 6 件の単体テストで lock-down。hooks/blog/hooks/career/のサブフォルダ化とCareerResumeForm.tsx(298 行) のsections/への JSX 分割 (Basic / Qualifications / SelfPr) で 198 行に縮小。lint / test (119 passed) / build すべて pass。Applied Changes
High
frontend/src/hooks/career/useProjectModalForm.ts:18-23:initProject(null)の 14 フィールド独立定義を削除し、structuredClone(blankCareerProject)に置換 (FE_report High force commit #1)。constants.ts:blankCareerProjectが唯一の SSoT となり、フィールド追加・改名は自動追従する。jscpd 16L クローン解消。Medium
frontend/src/payloadBuilders.test.ts: 27 行 → 280 行に拡充、26 テスト追加 (FE_report Medium force commit #1)。frontend/src/components/forms/CareerResumeForm.tsx: 298 行 → 198 行に縮小 (FE_report Medium ログイン #2)。3 つのセクションを切り出し:sections/CareerBasicInfoSection.tsx(53 行)sections/CareerQualificationsSection.tsx(102 行、資格 CRUD ハンドラ含む)sections/CareerSelfPrSection.tsx(35 行)frontend/src/components/forms/ProjectModal.tsx(274 行): 確認の結果、残り 274 行はほぼすべて JSX フォームフィールドで、state とハンドラは既にuseProjectModalFormに分離済み。現状維持と判断 (FE_report Medium Dev #3)。Low
useBlogAccountManager.ts254 行 / 意図的設計) と Low ログイン #2 (useCareerExperienceMutators.ts207 行 / ドメイン的に違う操作) は 現状維持 (FE_report Low)。reduceActionsのみ純粋関数として切り出してテスト可能にした (下記 Test Added)。Test Changes
Removed (= setup factory への置き換えによる重複解消、テスト本体は維持)
frontend/src/hooks/useDocumentForm.test.ts: 5 重複ペア (18L+26L+13L+15L+14L) のrenderHook+ props 標準セットをsetup(overrides, storeOverrides)ファクトリ関数に集約。UseDocumentFormOptions型を export して再利用。frontend/src/hooks/useTaskPolling.test.ts: 3 重複ペア (17L+15L+20L) をsetup(checkStatus, overrides)に集約。frontend/src/hooks/analysis/useAsyncAnalysisPage.test.ts: 4 重複ペアをsetup(overrides)に集約。UseAsyncAnalysisPageOptions<TResult>を export。frontend/src/hooks/career/useCareerAnalysisPage.test.ts: 2 重複ペアをsetup(initialAnalyses, mockOverrides)に集約。frontend/src/components/analysis/GitHubAnalysisPage.test.tsx: 2 重複ペア (mswserver.use空キャッシュ) をmockEmptyCache()ヘルパに集約。Added
frontend/src/payloadBuilders.test.ts: 5 → 26 テスト:validateDateRangeの境界 (start>end / start=end / is_current / 空文字)hasAnyTextの境界 (null/undefined/空白のみ/混在)buildCareerPayloadの必須項目バリデーション (氏名/職務要約/自己PR)buildCareerPayload (experiences)のis_current=true時 end_date null 化、空欄 filter 除外、start>end エラーbuildCareerPayload (projects/clients/team)のis_current=true時 end_date="" 化、has_client=false時 name="" 化、team.members空/有効要素のフィルタ、technology_stacks空 name 除外buildCareerPayload (qualifications)の片方欠落エラー / 空欄除外 / trimfrontend/src/hooks/blog/useBlogAccountManager.test.ts:reduceActionsの 6 件単体テスト追加 (FE_report Add Stg #4 = setAction の expectedAction ガード)。テストしやすくするためsetAction内部の純粋ロジックをreduceActions(prev, platform, action, expectedAction?)として export 化し、useState 用 reducer として直接検証可能にした。api/client.ts並行リフレッシュ flow) は既存client.test.ts184 行で auth/refresh の競合制御を網羅済み。Follow-ups に記載。Duplication Resolved
constants.ts:66-81↔useProjectModalForm.ts:22-36の 14 フィールド独立定義 (jscpd 16L):blankCareerProjectのstructuredClone参照に置換し SSoT 化。setup(overrides)ファクトリを導入して 1 箇所に集約。useBlogAccountManager.setActionの race-condition guard ロジック: useState の closure 内からreduceActions純粋関数として切り出し、単体テスト可能にした。残した偶発的重複 (Allowed Duplication)
useCareerExperienceMutators.ts内のsetForm((prev) => ({ ...prev, experiences: prev.experiences.map(...) }))パターン (jscpd 9L+8L+11L): ドメイン的に違う操作のため可読性優先で維持 (FE_report Low ログイン #2 と同判断)。e2e/auth.spec.tsの 10L クローン: E2E は明示的シナリオ手順を優先する方針 (FE_report Allowed)。Structure Changes
外部 import パスの追従先:
components/blog/BlogPage.tsx,BlogPlatformList.tsx→hooks/blog/useBlogAccountManagercomponents/forms/ProjectModal.tsx→hooks/career/useProjectModalFormcomponents/forms/sections/CareerExperienceSection.tsx→hooks/career/useCareerExperienceMutators,hooks/career/useProjectModalStatecomponents/career-analysis/CareerAnalysisPage.tsx→hooks/career/useCareerAnalysisPage../api→../../api,../types→../../types) も追従Skipped
useProjectModalFormに切り出し済みのため現状維持。useBlogAccountManager内部ヘルパ切り出し): per-platform lifecycle 集約という意図的設計のため現状維持。reduceActionsのみ純粋関数化してテスト可能にした。useCareerExperienceMutatorsの高階関数化): ドメイン的に違う操作を同じ抽象で扱うと読み手が混乱するため現状維持。client.tsの並行 401 リフレッシュ結合テスト): 既存client.test.ts184 行で auth/refresh 競合をすでに検証済み。並行ケースを更に厳格にしたい場合は別 PR。Validation
make lint-frontend: pass (eslint クリーン)make test-frontend: pass — 17 files / 119 tests passedmake build-frontend: pass — vite v6.4.2 build success (663 kB / gzip 207 kB)npm run test:e2e): 未実行。本 PR は API パス変更なし・新規ルート/レイアウト変更なし・サイドバー変更なし。JSX の責務分割のみで UI フローへの影響なし。.claude/rules/frontend/test.mdの E2E トリガー条件に該当しないため省略。Follow-ups
useNavigateを mock した分岐テスト追加。client.ts並行リフレッシュ): 複数 401 同時発火時にリフレッシュが 1 回に集約されるか検証。hooks/のフラット数: 残り 7 個 (useAuthSession / useDocumentForm / useMasterData / useNotifications / usePdfActions / useTaskPolling / useTheme)。15+ になればhooks/common/も検討。UseDocumentFormOptions/UseAsyncAnalysisPageOptionsを export 化したが、製品コードからの利用は無いため pure type 公開。テストドキュメント上の役割を明示するなら JSDoc 追加を検討。Summary by CodeRabbit
Refactor
Tests