Skip to content

Extract deep equality utility and refactor hooks to use shared implementation#273

Merged
yusuke0610 merged 6 commits into
mainfrom
dev
May 27, 2026
Merged

Extract deep equality utility and refactor hooks to use shared implementation#273
yusuke0610 merged 6 commits into
mainfrom
dev

Conversation

@yusuke0610

@yusuke0610 yusuke0610 commented May 26, 2026

Copy link
Copy Markdown
Owner

Summary

  • dirty 判定の isDeepEqualutils/deepEqual.ts に集約し、useCareerDirty / useProjectFormDirty の byte 一致コピペを解消(等価判定の乖離リスクを除去)。
  • useAsyncTaskPage の未参照デッドコード getNextInterval を削除(バックオフは useTaskPolling が実装済み)。
  • useCareerExperienceMutators に単体テストを新規追加し、フォーム state を書き換える仕様分岐(is_current↔end_date / has_client↔name / 最後の1件削除ガード / onProjectSave 追加・置換)を固定。
  • useTaskPolling.test のフレーキーな実時間待ち(setTimeout)を fake timers ベースへ置換し、unmount でポーリングが止まる仕様を決定論的に検証。

Applied Changes

High

  • なし(レポート通り構造的破綻なし)

Medium

  • [frontend/src/utils/deepEqual.ts:8] 新規。isDeepEqual を移設(Findings Medium / Duplication High「isDeepEqual の byte 一致コピペ」)。
  • [frontend/src/hooks/career/useCareerDirty.ts:10,117] ローカル定義を削除し ../../utils/deepEqual から import。挙動不変。
  • [frontend/src/hooks/career/useProjectFormDirty.ts:5,55-57] 同上。diffProject / team・technology_stacks・phases 比較はそのまま共通関数を利用。

Low

  • [frontend/src/hooks/useAsyncTaskPage.ts:21-25] getNextInterval の定義・export・「将来の拡張用」コメントを削除(Findings Low「デッドコード export」)。rg getNextInterval src/ で参照ゼロを確認済み。
  • [.claude/rules/frontend/architecture.md] hooks ディレクトリ記載 drift(Findings Low)を修正。実態調査で analyze 機能削除(commit ee481a0 "all anarize remove" 系)による drift が hooks 以外にも波及していたため、同種の乖離をまとめて是正:
    • hooks ブロックを実構成(top-level 8 件 + blog/ + career/)へ更新。存在しない useBlogSummaryPolling / useCareerAnalysisPage / useAsyncAnalysisPage / analysis/ 記載を削除し、未記載だった useAsyncTaskPage / useAuthSession を追記。
    • pages/ から削除済み CareerAnalysisPage.tsxcomponents/career-analysis/ ディレクトリ記載を削除。
    • api/ モジュール一覧から削除済み career-analysis / intelligence を除き、実在の githubLink を追記。
    • utils/ に新規 deepEqual.ts と既存未記載の pdfjs.ts / taskStatus.ts を追記。
    • 本文の非同期タスク進捗の記述を useAsyncAnalysisPageuseAsyncTaskPage に修正。

Test Changes

Removed

  • なし(レポートの削除推奨は無し)。

Added

  • [frontend/src/hooks/career/useCareerExperienceMutators.test.ts] 新規。守るユーザー挙動:
    • is_current=true で end_date がクリアされる / is_current=false では保持される
    • 通常フィールド更新は end_date を巻き込まない
    • has_client=false で取引先名がクリアされる / true では保持される
    • removeExperience / removeClient / removeProject の「最後の1件は削除せず blank で置換」ガード(各 1 件時)と複数時の index 削除
    • onProjectSave の projIndex=null(末尾追加)/ 非 null(置換)分岐
    • 実 React state(useState)越しに act で駆動し、操作後の form state を assert。

Changed

  • [frontend/src/hooks/useTaskPolling.test.ts:110-135] 「アンマウント時にポーリングが停止する」を置換。await new Promise((r)=>setTimeout(r, FAST_INTERVAL*3)) の実時間待ちを vi.useFakeTimers() + vi.advanceTimersByTimeAsync に変更。マウント中は反復(呼び出し ≥2)→ unmount → 仮想時間を進めても呼び出しが増えないことを決定論的に検証。守る仕様(unmount でポーリング停止)は維持。

Duplication Resolved

  • [frontend/src/utils/deepEqual.ts] Duplication Findings High「useCareerDirty.ts:72-96useProjectFormDirty.ts:30-54isDeepEqual(27L clone)」を統合。抽出先は duplication.md「Frontend → 純粋関数は src/utils/」に準拠。両 hook は import 1 行に置換し、再帰等価判定の正本を 1 か所に集約。
  • Allowed Duplication(useCareerExperienceMutators の nested setForm パターン、テストの arrange-act-assert 群)は偶発的重複として抽出せず維持(Skipped 参照)。

Structure Changes

  • utils/deepEqual.ts を新設したのみ。ディレクトリ移動・hook 切り出しなし(レポートの Oversized 3 件は全て「現状維持推奨」)。
frontend/src/
  utils/
    appError.ts
    errorId.ts
    pdfjs.ts
    taskStatus.ts
    deepEqual.ts   # ← 新規: isDeepEqual を useCareerDirty / useProjectFormDirty から集約

Skipped

  • Allowed Duplication 群(useCareerExperienceMutators の experience/client/project 別 nested updater、jscpd 検出のテスト arrange-act-assert 13 件)は duplication.md「許容される類似」に該当するため抽出しない。
  • Oversized Components(ProjectModal 321L / CareerExperienceEditor 296L / CareerResumeForm 272L)はレポート通り責務分離済み・分割は過剰抽象化になるため未対応。

Validation

  • make lint-frontend: pass(eslint src/ エラーなし)
  • make lint-frontend-messages: pass(grep ベース検知ゼロ)
  • make test-frontend: pass(node:test 4 / vitest 22 ファイル 160 tests 全 green。新規 useCareerExperienceMutators.test.ts 含む)
  • make build-frontend: pass(tsc -b + vite build、220 modules、型エラーなし)
  • E2E (npm run test:e2e): 未実行。理由: 新規ページ/ルート追加・認証/ナビゲーション/レイアウト/サイドバー変更・UI フローに影響する API 変更のいずれにも該当しない(util 抽出は挙動不変、デッドコード削除、テストのみの変更)。

Follow-ups

  • なし(architecture.md の drift 是正を本 PR に取り込み済み)。仕様判断が必要で保留にした項目もなし。

Summary by CodeRabbit

  • Tests

    • Added comprehensive tests for career form mutators and field behaviors
    • Improved task polling tests using fake timers to verify polling stops on unmount
  • Refactor

    • Introduced a shared deep-equality utility and migrated career/project dirty checks to use it
    • Removed an exported interval helper and simplified polling-related hook internals
  • Chores

    • Updated frontend architecture documentation to reflect current routing, hooks, task-statuses, and utilities

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: bc0f7b47-9d48-4474-b962-a41baa3192b0

📥 Commits

Reviewing files that changed from the base of the PR and between 8d55dc5 and bd6e83e.

📒 Files selected for processing (1)
  • .claude/rules/frontend/architecture.md
✅ Files skipped from review due to trivial changes (1)
  • .claude/rules/frontend/architecture.md

📝 Walkthrough

Walkthrough

Consolidates deep-equality into isDeepEqual, updates two career hooks to use it, adds comprehensive vitest coverage for career mutators, makes polling test deterministic and removes a polling export, updates frontend architecture docs, and changes a backend FastAPI pin.

Changes

Career form hooks refactoring and polling infrastructure improvements

Layer / File(s) Summary
Shared deep-equality utility
frontend/src/utils/deepEqual.ts
isDeepEqual utility added to perform recursive deep comparisons for arrays and plain objects.
Career form hooks refactored to use shared utility
frontend/src/hooks/career/useCareerDirty.ts, frontend/src/hooks/career/useProjectFormDirty.ts
Both hooks remove file-local deep-equality implementations and import isDeepEqual from the new utility while preserving dirty-field logic.
Test coverage for career experience mutators
frontend/src/hooks/career/useCareerExperienceMutators.test.ts
New Vitest tests cover updateExperienceField, updateClientHasClient, removeExperience, removeClient, removeProject, and onProjectSave behaviors including boundary cases.
Polling test determinism and API cleanup
frontend/src/hooks/useTaskPolling.test.ts, frontend/src/hooks/useAsyncTaskPage.ts
Unmount polling test rewritten to use fake timers and getNextInterval export removed from useAsyncTaskPage.
Architecture documentation updates
.claude/rules/frontend/architecture.md
Docs updated: route/component list (CareerPage/BlogPage/GitHubLinkPage), hooks overview (useTaskPolling, useAsyncTaskPage, auth/theme hooks), and utils reorganized.

Backend dependency update

Layer / File(s) Summary
FastAPI version pin
backend/requirements.txt
Pinned fastapi downgraded from 0.136.3 to 0.136.1.

Possibly related PRs

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A tiny helper hops into the stack,
One deep-equal bridge to keep forms on track,
Tests snugly watch each mutator's move,
Timers faked so polling won't misprove,
Docs and pins adjusted — nibble, patch, and pack!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: extracting a shared deep equality utility and refactoring hooks to use it, which is the primary focus of the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 @.claude/rules/frontend/architecture.md:
- Line 74: The doc's status list is narrower than the implementation: update the
rule text so it matches frontend/src/hooks/useAsyncTaskPage.ts by either adding
"pending" to the enumerated statuses (dead_letter / processing / completed /
pending) or explicitly mark the list as "example statuses" to avoid mismatch;
reference useAsyncTaskPage and TaskProgressStepper in the sentence so readers
know the canonical source of statuses and ensure the documentation and
implementation remain aligned.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a6122172-e615-40da-9d66-9f5b7718d302

📥 Commits

Reviewing files that changed from the base of the PR and between 6889675 and ec66f91.

📒 Files selected for processing (7)
  • .claude/rules/frontend/architecture.md
  • frontend/src/hooks/career/useCareerDirty.ts
  • frontend/src/hooks/career/useCareerExperienceMutators.test.ts
  • frontend/src/hooks/career/useProjectFormDirty.ts
  • frontend/src/hooks/useAsyncTaskPage.ts
  • frontend/src/hooks/useTaskPolling.test.ts
  • frontend/src/utils/deepEqual.ts
💤 Files with no reviewable changes (1)
  • frontend/src/hooks/useAsyncTaskPage.ts

Comment thread .claude/rules/frontend/architecture.md Outdated
@yusuke0610 yusuke0610 changed the title dev Extract deep equality utility and refactor hooks to use shared implementation May 26, 2026
@yusuke0610
yusuke0610 merged commit 2b03523 into main May 27, 2026
31 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant