Skip to content

地下鉄区間では速度フィルタ・EMAスムージングを全てスキップする#5664

Merged
TinyKitten merged 2 commits intodevfrom
claude/review-sgd-algorithm-R2vYX
Mar 24, 2026
Merged

地下鉄区間では速度フィルタ・EMAスムージングを全てスキップする#5664
TinyKitten merged 2 commits intodevfrom
claude/review-sgd-algorithm-R2vYX

Conversation

@TinyKitten
Copy link
Copy Markdown
Member

@TinyKitten TinyKitten commented Mar 24, 2026

Summary by CodeRabbit

  • バグ修正
    • 特定条件(スムージングをスキップする設定や前回位置がない場合)で位置情報を即座に記録するよう改善。
    • 不要なフィルター処理を回避し、精度履歴の更新を確実に行うように変更。
    • 位置の異常値判定とスムージングは、前回位置がありスムージングが有効な場合のみ適用されるよう明確化。

GPS信号が不安定な地下鉄では速度ベースの異常値棄却も無意味なため、
早期リターンでフィルタ処理全体をバイパスする

https://claude.ai/code/session_01JU67LRYTFiwHjDt9jXMQCb
@github-actions github-actions bot added the react label Mar 24, 2026
@TinyKitten TinyKitten changed the title 地下鉄区間では速度フィルタ・EMAスムージングを全てスキップする 地下鉄区間では速度フィルタをスキップする Mar 24, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 24, 2026

📝 Walkthrough

Walkthrough

src/store/atoms/location.ts の制御フローを再構成し、skipSmoothing が true または prev が null の場合に早期リターンを追加。この場合、速度ベースのフィルタリングと EMA スムージングをバイパスして生のロケーションを直接保存します。

Changes

Cohort / File(s) Summary
ロケーション管理の制御フロー再構成
src/store/atoms/location.ts
skipSmoothing が true または prev が null の場合に早期リターンを導入。これらのパスでは locationAtomaccuracyHistoryAtom に raw ロケーションを格納し、速度ベースのアウトライア判定と EMA スムージングを実行しないように変更。prev != null かつ skipSmoothing が false の場合のみ速度チェックと EMA が適用されるよう整理。

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 ぴょんと跳ねて、分岐を渡る
早期リターンで道を短くする
生の座標、そっと抱えて
スピードと EMA は別れ道へ
軽やかに、でも確かに進む

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed プルリクエストのタイトルは、変更内容の主要な目的を明確に説明しており、地下鉄区間で速度フィルタとEMAスムージングをスキップするという具体的な変更内容を正確に反映しています。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/review-sgd-algorithm-R2vYX

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

@TinyKitten TinyKitten changed the title 地下鉄区間では速度フィルタをスキップする 地下鉄区間では速度フィルタ・EMAスムージングを全てスキップする Mar 24, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/store/atoms/location.ts (1)

40-42: コメントの更新が必要です。

Line 40のコメントは「EMAスムージングを無効化する」と記載されていますが、実際の動作は速度フィルタも含めて全てスキップするように変更されています。Line 44のコメントと一致させるため、更新を検討してください。

-  // 地下鉄ではGPS信号が不安定なためEMAスムージングを無効化する
+  // 地下鉄ではGPS信号が不安定なためフィルタ・スムージングを全てスキップする
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/store/atoms/location.ts` around lines 40 - 42, コメントが実際の挙動と一致していません:
currentLineType、skipSmoothing、stationState、LineType.Subway の判定は EMA
スムージングだけでなく速度フィルタも含めて「全てスキップ」する判定なので、該当コメントを「EMA
スムージングと速度フィルタを含む位置フィルタ処理をスキップする」等に書き換えて、Line 44 の説明と整合させてください。
🤖 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/store/atoms/location.ts`:
- Around line 40-42: コメントが実際の挙動と一致していません:
currentLineType、skipSmoothing、stationState、LineType.Subway の判定は EMA
スムージングだけでなく速度フィルタも含めて「全てスキップ」する判定なので、該当コメントを「EMA
スムージングと速度フィルタを含む位置フィルタ処理をスキップする」等に書き換えて、Line 44 の説明と整合させてください。

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0da021d3-e8c0-4097-ab4c-58841cffd6f0

📥 Commits

Reviewing files that changed from the base of the PR and between e188f85 and 23bdb9f.

📒 Files selected for processing (1)
  • src/store/atoms/location.ts

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

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 the current code and only fix it if needed.

Inline comments:
In `@src/store/atoms/location.ts`:
- Around line 44-49: Currently when skipSmoothing is true you write the raw
subway coordinate into locationAtom (which becomes prev) and that noisy prev can
cause the very next non-subway fix to be rejected by the MAX_PLAUSIBLE_SPEED
check; change this by separating the display value from the “previous for
speed/filtering” value: keep writing the raw coordinate to
locationAtom/accuracyHistoryAtom for UI, but do NOT update prev (or maintain a
separate lastFilteredPrev/lastForSpeed atom) while skipSmoothing is true;
additionally detect the transition skipSmoothing→false and skip the speed
plausibility check for the first non-skipped sample (or clear lastForSpeed so
the first non-skipped update treats prev==null), so functions/atoms involved are
locationAtom, accuracyHistoryAtom, prev (or new lastForSpeed atom),
skipSmoothing, and the MAX_PLAUSIBLE_SPEED speed check logic.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 27ebe94f-066b-4f27-b62d-6f016ff3a925

📥 Commits

Reviewing files that changed from the base of the PR and between 23bdb9f and e9cb9a9.

📒 Files selected for processing (1)
  • src/store/atoms/location.ts

@TinyKitten TinyKitten merged commit 481eaa3 into dev Mar 24, 2026
6 checks passed
@TinyKitten TinyKitten deleted the claude/review-sgd-algorithm-R2vYX branch March 24, 2026 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants