GPSで逆方向走行を検知しユーザーに通知する機能を追加#5734
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough位置情報の連続的な次駅距離増加を検出する新フック Changes
Sequence Diagram(s)sequenceDiagram
participant GPS as GPS/Location
participant Detector as useWrongDirectionDetector
participant WarningInfo as useWarningInfo
participant Refresh as useRefreshStation
participant UI as Notification/UI
GPS->>Detector: 位置情報更新 (lat, lon, accuracy)
Detector->>Detector: 次駅距離計算・前回距離と比較
Detector->>Detector: 連続増加カウントと累積増加を更新
alt 閾値超過 (連続増加≥4 かつ 累積増加≥300m)
Detector-->>WarningInfo: isWrongDirection / isLoopLineWrongDirection = true
Detector-->>Refresh: isWrongDirection / isLoopLineWrongDirection = true
Refresh->>Refresh: wrongDirectionNotifiedRef を確認して通知送信
Refresh->>UI: 通知表示 (translate('wrongDirectionNotificationTitle') / translate(bodyKey))
WarningInfo->>UI: 警告表示 (translate(bodyKey), URGENT/WARNING)
else 閾値未満または解除
Detector->>Detector: カウンタと状態をリセット
Refresh->>Refresh: wrongDirectionNotifiedRef を必要に応じリセット
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/hooks/useWrongDirectionDetector.ts (2)
92-97:nextStationが変更された場合のprevDistanceRefの扱い
nextStationが変更されると、prevDistanceRefには古い駅への距離が残った状態で新しい駅への距離と比較されます。最初の測定で大きな距離差が生じ、誤検知の可能性があります。
nextStation.idの変更時にprevDistanceRefをリセットすることを検討してください。🔧 nextStation 変更時のリセット追加
+ const nextStationId = nextStation?.id; + useEffect(() => { + // nextStationが変わったら前回距離をリセット + prevDistanceRef.current = null; + }, [nextStationId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useWrongDirectionDetector.ts` around lines 92 - 97, When nextStation changes the hook still compares the new station distance against prevDistanceRef.current (leftover from the old station) causing false positives; update the hook (useWrongDirectionDetector) to detect changes to nextStation.id and reset prevDistanceRef.current (and optionally wrongDirectionDetectedRef.current) when nextStation.id changes so the first measurement for the new station is treated as initial (no comparison) rather than compared to the previous station's distance.
64-133:useMemo内での ref 変更は動作するが、副作用を持つパターン
useMemoは純粋な計算を想定していますが、このコードでは ref を変更しています(Lines 93, 102-108, 117-122)。これは React の期待する使用法からは外れていますが、ref は再レンダーをトリガーしないため実際には動作します。将来のメンテナンス性のため、この副作用を
useEffectに分離するか、コメントでこのパターンの意図を明示することを検討してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useWrongDirectionDetector.ts` around lines 64 - 133, The memoized computation is mutating refs (prevDistanceRef, consecutiveIncreaseCountRef, cumulativeIncreaseRef, wrongDirectionDetectedRef, notifiedForStationIdRef) inside isWrongDirectionRaw's useMemo which introduces hidden side-effects; extract all ref updates and stateful logic into a separate useEffect that depends on [accuracy, autoModeEnabled, latitude, longitude, nextStation, selectedBound] and have useMemo only compute and return the pure boolean based on those refs/state (or alternatively keep the current mutation but add a clear comment above isWrongDirectionRaw explaining the intentional ref mutations), updating functions/variables named in this diff (isWrongDirectionRaw, prevDistanceRef, consecutiveIncreaseCountRef, cumulativeIncreaseRef, wrongDirectionDetectedRef, notifiedForStationIdRef) accordingly so no ref writes occur inside useMemo.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useRefreshStation.ts`:
- Around line 187-203: Replace the hardcoded notification title/body in
useEffect inside useRefreshStation.ts so it uses the translation keys from
useWarningInfo.ts: call the i18n/translation function (same one used to expose
wrongDirectionWarning and wrongDirectionLoopLineWarning) and pass the correct
key depending on isLoopLineWrongDirection; keep using sendNotificationAsync and
wrongDirectionNotifiedRef logic but select wrongDirectionLoopLineWarning when
isLoopLineWrongDirection is true and wrongDirectionWarning otherwise, and ensure
the title also comes from translations instead of hardcoded strings.
---
Nitpick comments:
In `@src/hooks/useWrongDirectionDetector.ts`:
- Around line 92-97: When nextStation changes the hook still compares the new
station distance against prevDistanceRef.current (leftover from the old station)
causing false positives; update the hook (useWrongDirectionDetector) to detect
changes to nextStation.id and reset prevDistanceRef.current (and optionally
wrongDirectionDetectedRef.current) when nextStation.id changes so the first
measurement for the new station is treated as initial (no comparison) rather
than compared to the previous station's distance.
- Around line 64-133: The memoized computation is mutating refs
(prevDistanceRef, consecutiveIncreaseCountRef, cumulativeIncreaseRef,
wrongDirectionDetectedRef, notifiedForStationIdRef) inside isWrongDirectionRaw's
useMemo which introduces hidden side-effects; extract all ref updates and
stateful logic into a separate useEffect that depends on [accuracy,
autoModeEnabled, latitude, longitude, nextStation, selectedBound] and have
useMemo only compute and return the pure boolean based on those refs/state (or
alternatively keep the current mutation but add a clear comment above
isWrongDirectionRaw explaining the intentional ref mutations), updating
functions/variables named in this diff (isWrongDirectionRaw, prevDistanceRef,
consecutiveIncreaseCountRef, cumulativeIncreaseRef, wrongDirectionDetectedRef,
notifiedForStationIdRef) accordingly so no ref writes occur inside useMemo.
🪄 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: 94c1e111-b9fc-4e23-b6ab-f5ade569ec56
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
assets/translations/en.jsonassets/translations/ja.jsonsrc/hooks/index.tssrc/hooks/useRefreshStation.test.tsxsrc/hooks/useRefreshStation.tssrc/hooks/useWarningInfo.tssrc/hooks/useWrongDirectionDetector.ts
- 通知タイトル・本文をハードコードからtranslateキーに変更 - nextStation変更時にprevDistanceRefをリセットし誤検知を防止 - useMemo内のref副作用をuseEffectに移行しstate管理をuseStateに変更 https://claude.ai/code/session_017pSrj2MtyLMKMMSwhG8A6h
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 `@src/hooks/useRefreshStation.ts`:
- Around line 195-199: sendNotificationAsync is returning a Promise but here
it's called without handling — update the block that calls
sendNotificationAsync({ title: translate('wrongDirectionNotificationTitle'),
body: translate(bodyKey) }) so the Promise is explicitly handled (match the
pattern used earlier around the await at line ~143): either await the call
inside the surrounding async function with try/catch and log or handle errors,
or append .catch(...) to the Promise; ensure wrongDirectionNotifiedRef.current =
true still runs only after the notification call resolves or in the finally
branch as appropriate.
In `@src/hooks/useWrongDirectionDetector.ts`:
- Around line 67-82: The early-return checks in the useWrongDirectionDetector
hook (conditions around selectedBound, autoModeEnabled, latitude/longitude,
nextStation, and accuracy vs BAD_ACCURACY_THRESHOLD) do not reset
wrongDirectionDetected or its counter, so prior true states can persist; update
the hook so that whenever any of those preconditions fail you explicitly reset
wrongDirectionDetected (and the related counter/state used for debouncing) to
false/zero before returning, referencing the wrongDirectionDetected state and
the counter/reset function used in this hook to ensure no stale detection
remains (e.g., on autoModeEnabled flips or loss of GPS/nextStation data).
🪄 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: 5734017a-c2f6-4d70-8cd6-eadc6ab376c1
📒 Files selected for processing (4)
assets/translations/en.jsonassets/translations/ja.jsonsrc/hooks/useRefreshStation.tssrc/hooks/useWrongDirectionDetector.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- assets/translations/en.json
- assets/translations/ja.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useWrongDirectionDetector.ts`:
- Around line 78-81: When clearing the wrong-direction state in
useWrongDirectionDetector (calls to setWrongDirectionDetected(false) that also
reset consecutiveIncreaseCountRef, cumulativeIncreaseRef and prevDistanceRef),
also reset notifiedForStationIdRef (e.g. set notifiedForStationIdRef.current =
null) so the same nextStation.id can trigger detection again; update all reset
sites that clear wrongDirectionDetected (the blocks that touch
consecutiveIncreaseCountRef, cumulativeIncreaseRef, prevDistanceRef and call
setWrongDirectionDetected) to also clear notifiedForStationIdRef to allow
re-notification after a reset.
🪄 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: c04c3cd5-06fa-41bc-916f-8f41013b774d
📒 Files selected for processing (2)
src/hooks/useRefreshStation.tssrc/hooks/useWrongDirectionDetector.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/hooks/useRefreshStation.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useWrongDirectionDetector.ts`:
- Around line 44-51: The arrived-effect in useWrongDirectionDetector fails to
clear notifiedForStationIdRef on arrival; update the useEffect that runs when
arrived (the block that currently resets prevDistanceRef.current,
consecutiveIncreaseCountRef.current, cumulativeIncreaseRef.current and
setWrongDirectionDetected(false)) to also set notifiedForStationIdRef.current =
null so the arrival reset path matches the other reset paths (lines referencing
notifiedForStationIdRef at other reset points).
🪄 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: a65e25ec-1d58-4668-8593-e82883dd0025
📒 Files selected for processing (1)
src/hooks/useWrongDirectionDetector.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/hooks/useWrongDirectionDetector.ts (1)
135-142: 依存配列は stable なプリミティブに寄せた方が安全です。
selectedBound/nextStationの参照だけが作り直された場合でも effect が再実行され、同じ位置の再計測で Line 116-120 のリセットに入ります。selectedBoundIdとnextStation?.id / latitude / longitudeに絞ると、位置更新以外の誤リセットを避けやすいです。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useWrongDirectionDetector.ts` around lines 135 - 142, The effect in useWrongDirectionDetector is currently depending on object references (selectedBound and nextStation) which can trigger spurious reruns and cause the reset logic around lines 116-120 to run unnecessarily; update the dependency array to use stable primitives instead: replace selectedBound and nextStation with selectedBoundId and nextStation?.id (and keep latitude and longitude and accuracy/autoModeEnabled as needed) so the effect only reruns on meaningful changes to selectedBoundId, nextStation id, latitude, longitude, accuracy, and autoModeEnabled; ensure you use optional chaining for nextStation?.id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useWrongDirectionDetector.ts`:
- Around line 37-39: notifiedForStationIdRef currently uses null as the “unset”
sentinel which collides with a legitimate nullable Station.id; change its type
to allow undefined (e.g., useRef<number | null | undefined>(undefined)) so
undefined means “not set” and null remains a valid station id, then update the
guard that compares notifiedForStationIdRef.current === nextStation.id (and any
related logic in useWrongDirectionDetector, including setWrongDirectionDetected
usage) to only short-circuit when notifiedForStationIdRef.current is !==
undefined (or explicitly check for undefined) so a nextStation.id of null does
not suppress the first detection.
---
Nitpick comments:
In `@src/hooks/useWrongDirectionDetector.ts`:
- Around line 135-142: The effect in useWrongDirectionDetector is currently
depending on object references (selectedBound and nextStation) which can trigger
spurious reruns and cause the reset logic around lines 116-120 to run
unnecessarily; update the dependency array to use stable primitives instead:
replace selectedBound and nextStation with selectedBoundId and nextStation?.id
(and keep latitude and longitude and accuracy/autoModeEnabled as needed) so the
effect only reruns on meaningful changes to selectedBoundId, nextStation id,
latitude, longitude, accuracy, and autoModeEnabled; ensure you use optional
chaining for nextStation?.id.
🪄 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: 9b1f0f77-b849-46c8-84fd-943d19c554b1
📒 Files selected for processing (1)
src/hooks/useWrongDirectionDetector.ts
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/hooks/useWrongDirectionDetector.ts (1)
92-98:⚠️ Potential issue | 🟠 Major次駅切替時にも検知状態を完全にリセットしてください。
ここではカウンタと距離だけを戻していますが、
wrongDirectionDetectedとnotifiedForStationIdRefは前駅の値が残ります。nextStationIdが変わった直後も前駅の警告が残り、ID が戻ったケースでは再検知も抑止されるので、この分岐でも両方クリアした方が安全です。修正案
if (prevNextStationIdRef.current !== nextStationId) { prevNextStationIdRef.current = nextStationId; prevDistanceRef.current = null; consecutiveIncreaseCountRef.current = 0; cumulativeIncreaseRef.current = 0; + notifiedForStationIdRef.current = undefined; + setWrongDirectionDetected(false); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useWrongDirectionDetector.ts` around lines 92 - 98, When nextStationId changes (inside the branch where prevNextStationIdRef.current !== nextStationId) also reset the detection state: set wrongDirectionDetected to false (or its ref equivalent) and clear notifiedForStationIdRef.current (e.g., set to null/undefined) so previous-station warnings don't persist and suppress future detections for the new station; update the block that currently resets prevDistanceRef.current, consecutiveIncreaseCountRef.current, and cumulativeIncreaseRef.current to include clearing wrongDirectionDetected and notifiedForStationIdRef.current as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/hooks/useWrongDirectionDetector.ts`:
- Around line 92-98: When nextStationId changes (inside the branch where
prevNextStationIdRef.current !== nextStationId) also reset the detection state:
set wrongDirectionDetected to false (or its ref equivalent) and clear
notifiedForStationIdRef.current (e.g., set to null/undefined) so
previous-station warnings don't persist and suppress future detections for the
new station; update the block that currently resets prevDistanceRef.current,
consecutiveIncreaseCountRef.current, and cumulativeIncreaseRef.current to
include clearing wrongDirectionDetected and notifiedForStationIdRef.current as
well.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 28518504-01fc-4da3-9c23-4ffd0e565170
📒 Files selected for processing (1)
src/hooks/useWrongDirectionDetector.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/hooks/useWrongDirectionDetector.ts (1)
52-56: リセット処理はヘルパー化すると保守しやすくなります。同一ロジックが複数箇所にあるため、将来の修正漏れ防止のために共通化を検討してください。
refactor案
+ const resetDetectionState = () => { + prevDistanceRef.current = null; + consecutiveIncreaseCountRef.current = 0; + cumulativeIncreaseRef.current = 0; + notifiedForStationIdRef.current = undefined; + setWrongDirectionDetected(false); + }; + useEffect(() => { if (arrived) { - prevDistanceRef.current = null; - consecutiveIncreaseCountRef.current = 0; - cumulativeIncreaseRef.current = 0; - notifiedForStationIdRef.current = undefined; - setWrongDirectionDetected(false); + resetDetectionState(); } }, [arrived]); useEffect(() => { if (selectedBoundId != null) { - prevDistanceRef.current = null; - consecutiveIncreaseCountRef.current = 0; - cumulativeIncreaseRef.current = 0; - notifiedForStationIdRef.current = undefined; - setWrongDirectionDetected(false); + resetDetectionState(); } }, [selectedBoundId]);Also applies to: 63-67, 84-88, 95-99, 124-127
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useWrongDirectionDetector.ts` around lines 52 - 56, The repeated reset logic in the useWrongDirectionDetector hook (setting prevDistanceRef.current = null, consecutiveIncreaseCountRef.current = 0, cumulativeIncreaseRef.current = 0, notifiedForStationIdRef.current = undefined, and setWrongDirectionDetected(false)) should be extracted into a single helper function (e.g., resetWrongDirectionState or resetDetector) inside the hook; replace each duplicate block (the other occurrences in the hook where the same five assignments appear) with a call to this helper and ensure any callers still pass needed context or params if required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/hooks/useWrongDirectionDetector.ts`:
- Around line 52-56: The repeated reset logic in the useWrongDirectionDetector
hook (setting prevDistanceRef.current = null,
consecutiveIncreaseCountRef.current = 0, cumulativeIncreaseRef.current = 0,
notifiedForStationIdRef.current = undefined, and
setWrongDirectionDetected(false)) should be extracted into a single helper
function (e.g., resetWrongDirectionState or resetDetector) inside the hook;
replace each duplicate block (the other occurrences in the hook where the same
five assignments appear) with a call to this helper and ensure any callers still
pass needed context or params if required.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 096150e8-243e-44f7-9978-c8fadbb1972d
📒 Files selected for processing (1)
src/hooks/useWrongDirectionDetector.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/hooks/useWrongDirectionDetector.ts (1)
126-132: 軽微な冗長性:?? nullは不要ですLine 80 のガード
nextStationId === undefinedにより、この時点でnextStationIdはundefinedになり得ないため、?? nullフォールバックは冗長です。ただし、防御的なコードとして残しても害はありません。♻️ 省略可能な修正
- notifiedForStationIdRef.current = nextStationId ?? null; + notifiedForStationIdRef.current = nextStationId;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useWrongDirectionDetector.ts` around lines 126 - 132, The assignment to notifiedForStationIdRef.current uses a redundant nullish coalescing fallback (?? null) even though earlier guard ensures nextStationId cannot be undefined; remove the fallback and set notifiedForStationIdRef.current = nextStationId instead. Update the code around the notifiedForStationIdRef and nextStationId check in useWrongDirectionDetector so the final assignment is direct (notifiedForStationIdRef.current = nextStationId) to eliminate the unnecessary ?? null while preserving the existing guard logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/hooks/useWrongDirectionDetector.ts`:
- Around line 126-132: The assignment to notifiedForStationIdRef.current uses a
redundant nullish coalescing fallback (?? null) even though earlier guard
ensures nextStationId cannot be undefined; remove the fallback and set
notifiedForStationIdRef.current = nextStationId instead. Update the code around
the notifiedForStationIdRef and nextStationId check in useWrongDirectionDetector
so the final assignment is direct (notifiedForStationIdRef.current =
nextStationId) to eliminate the unnecessary ?? null while preserving the
existing guard logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dce2dc45-aceb-48e2-8a2e-45b2456f193d
📒 Files selected for processing (1)
src/hooks/useWrongDirectionDetector.ts
https://claude.ai/code/session_017pSrj2MtyLMKMMSwhG8A6h
Summary by CodeRabbit