Conversation
|
Note
|
| コホート / ファイル | 変更概要 |
|---|---|
ローカル停止駅配列の導入と再利用 src/components/SelectBoundModal.tsx |
handleBoundSelected内でstopsを導入。terminateBySelectedStationがtrueの場合はgetTerminatedStations(selectedStation, stations)を用いてstopsを算出し、以降のstations状態更新・境界(inbound/outbound)選択ロジックでそのstopsを再利用。oedoLineTerminusへの直接参照を除去し、依存にgetTerminatedStationsを含めるよう調整。 |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
注視すべき領域:
stopsが常に terminated stations または元のstationsで適切に初期化されること- INBOUND 時の
stops[stops.length - 1]と OUTBOUND 時のstops[0]が期待通りの駅を指すこと getTerminatedStationsの呼び出しが副作用や参照整合性に影響を与えていないこと
Possibly related PRs
- 経路検索から終点を指定&経路検索のバグを修正 #3994: 似たく終着駅選択ロジックを導入/変更し、
getTerminatedStations(または類似ヘルパー)を使用しているため関連性が高い。 - 直通先計算で終点駅が直通先の次の駅に接続していない場合、実質接続していない路線は省く #4007: 境界選択で計算済みの終着駅配列を使う変更を行っており、bound選択ロジックの扱いが類似しているため関連。
- 「この駅に向かう」ボタン実装 #4687: handleBoundSelected周りの境界選択フローと終着判定の変更を含むためコードレベルで関連する可能性あり。
Poem
🐰 ローカルなstopsで、
計算は一度きり、
駅たち整列、道しるべ、
コードはすっきり、ぴょんと行く 🚆
Pre-merge checks and finishing touches
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | プルリクエストのタイトル「終着駅指定のバグを修正」は、変更内容と一致しており、行先指定時の終着駅表示に関するバグ修正という主な変更を明確に示しています。 |
| Linked Issues check | ✅ Passed | コード変更は、getTerminatedStations() の使用を導入し、選択された駅に基づいて終着駅のロジックを適用することで、Issue #4695 の要件を満たしています。 |
| Out of Scope Changes check | ✅ Passed | すべての変更は SelectBoundModal.tsx 内の終着駅指定ロジックに限定されており、Issue #4695 のスコープ内です。 |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
fix/select-destination
📜 Recent review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/SelectBoundModal.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/{components,screens}/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Component files should use PascalCase naming (e.g.,
StationBanner.tsx)Use PascalCase for component file names (e.g.,
StationBanner.tsx)
Files:
src/components/SelectBoundModal.tsx
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Biome as the formatter for all code files
Files:
src/components/SelectBoundModal.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Use Biome as the linter for all code files
Use// biome-ignoreonly for true compatibility issues and include an inline reasonUse
.editorconfigstandards: UTF-8 encoding, two-space indentation, single quotes, and ES5 trailing commas
Files:
src/components/SelectBoundModal.tsx
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Biome is authoritative for linting; avoid
// biome-ignoreunless a rule is truly incompatible and document the rationale inline
Files:
src/components/SelectBoundModal.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: TinyKitten
Repo: TrainLCD/MobileApp PR: 4448
File: functions/src/funcs/tts.ts:214-232
Timestamp: 2025-08-11T20:23:23.071Z
Learning: PR #4448 in TrainLCD/MobileApp is specifically about AI feedback summarization and labeling functionality, not about TTS (Text-to-Speech) improvements. The TTS code changes appear to be part of a modularization refactoring but are not the focus of this PR.
Learnt from: TinyKitten
Repo: TrainLCD/MobileApp PR: 0
File: :0-0
Timestamp: 2025-08-11T21:13:39.083Z
Learning: TinyKitten prefers concise code reviews for the TrainLCD/MobileApp repository. Keep review comments brief and to the point.
📚 Learning: 2025-05-19T10:59:37.813Z
Learnt from: TinyKitten
Repo: TrainLCD/MobileApp PR: 4286
File: src/components/HeaderLED.tsx:5-10
Timestamp: 2025-05-19T10:59:37.813Z
Learning: The hooks useCurrentStation, useNextStation, useIsNextLastStop, and useNumbering are properly exported in src/hooks/index.ts, allowing them to be imported from '../hooks' in src/components/HeaderLED.tsx as implemented in PR #4286.
Applied to files:
src/components/SelectBoundModal.tsx
🔇 Additional comments (2)
src/components/SelectBoundModal.tsx (2)
184-194: 依存配列の更新は正確。
getTerminatedStationsを依存配列に追加したことで、useCallbackの依存関係が正しく保たれています。
156-170: Based on my verification, I can now generate the rewritten review comment. The code fix is correct and the edge case concern, while theoretically valid, is mitigated by defensive programming practices in the codebase.
Bug fix correctly ensures consistency between stations and selectedBound.
The introduction of the
stopsvariable ensures thatselectedBoundis calculated from the same terminated station list used for thestationsstate, fixing the bug where these could previously be out of sync.The theoretical edge case where
getTerminatedStationsreturns an empty array (causingstops[0]orstops[stops.length - 1]to beundefined) is unlikely in practice given the function's implementation: it returns either the fullstationsFromArgsor a slice containing at least one element when indices are found. Additionally, all downstream usages ofselectedBoundthroughout the codebase employ optional chaining (selectedBound?.id,selectedBound?.groupId, etc.), which safely handles any unexpectedundefinedvalues.The dependency array update at line 192 is correct and includes
getTerminatedStations.
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/SelectBoundModal.tsx (1)
158-176:stops導入でバグは解消されていそうですが、大江戸線条件と空配列時のフォールバックを確認したいです終着駅指定時に
stationsとselectedBoundを同じstopsから計算するようにしたのは筋が良く、PR 説明のバグはこれで解消されていそうです。一方で、
const stops = terminateBySelectedStation ? getTerminatedStations(selectedStation, stations) : stations; selectedBound: line?.id === TOEI_OEDO_LINE_ID && !stops.length ? oedoLineTerminus : direction === 'INBOUND' ? stops[stops.length - 1] : stops[0];という条件だと、通常は
stopsが空になることはなく、大江戸線特例 (oedoLineTerminus) の分岐がほぼ走らないように見えます。大江戸線だけ従来の終着駅ロジックを残したい意図がある場合は、ここを!terminateBySelectedStationなど別条件で見るべきかどうか、仕様的に一度確認しておくと安心です。また、理論上
stopsが空配列になった場合にstops[stops.length - 1]/stops[0]がundefinedになり得るので、念のためdirection === 'INBOUND' ? stops[stops.length - 1] ?? selectedStation : stops[0] ?? selectedStationのようにフォールバックを入れておくと、防御的で安全だと思います(必要であれば大江戸線用の条件もこの形に組み込めます)。
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/SelectBoundModal.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/{components,screens}/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Component files should use PascalCase naming (e.g.,
StationBanner.tsx)Use PascalCase for component file names (e.g.,
StationBanner.tsx)
Files:
src/components/SelectBoundModal.tsx
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Biome as the formatter for all code files
Files:
src/components/SelectBoundModal.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Use Biome as the linter for all code files
Use// biome-ignoreonly for true compatibility issues and include an inline reasonUse
.editorconfigstandards: UTF-8 encoding, two-space indentation, single quotes, and ES5 trailing commas
Files:
src/components/SelectBoundModal.tsx
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Biome is authoritative for linting; avoid
// biome-ignoreunless a rule is truly incompatible and document the rationale inline
Files:
src/components/SelectBoundModal.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: TinyKitten
Repo: TrainLCD/MobileApp PR: 4448
File: functions/src/funcs/tts.ts:214-232
Timestamp: 2025-08-11T20:23:23.071Z
Learning: PR #4448 in TrainLCD/MobileApp is specifically about AI feedback summarization and labeling functionality, not about TTS (Text-to-Speech) improvements. The TTS code changes appear to be part of a modularization refactoring but are not the focus of this PR.
Learnt from: TinyKitten
Repo: TrainLCD/MobileApp PR: 4286
File: src/components/HeaderLED.tsx:5-10
Timestamp: 2025-05-19T10:59:37.813Z
Learning: The hooks useCurrentStation, useNextStation, useIsNextLastStop, and useNumbering are properly exported in src/hooks/index.ts, allowing them to be imported from '../hooks' in src/components/HeaderLED.tsx as implemented in PR #4286.
Learnt from: TinyKitten
Repo: TrainLCD/MobileApp PR: 0
File: :0-0
Timestamp: 2025-08-11T21:13:39.083Z
Learning: TinyKitten prefers concise code reviews for the TrainLCD/MobileApp repository. Keep review comments brief and to the point.
📚 Learning: 2025-05-19T10:59:37.813Z
Learnt from: TinyKitten
Repo: TrainLCD/MobileApp PR: 4286
File: src/components/HeaderLED.tsx:5-10
Timestamp: 2025-05-19T10:59:37.813Z
Learning: The hooks useCurrentStation, useNextStation, useIsNextLastStop, and useNumbering are properly exported in src/hooks/index.ts, allowing them to be imported from '../hooks' in src/components/HeaderLED.tsx as implemented in PR #4286.
Applied to files:
src/components/SelectBoundModal.tsx
🧬 Code graph analysis (1)
src/components/SelectBoundModal.tsx (1)
src/constants/line.ts (1)
TOEI_OEDO_LINE_ID(4-4)
fixes #4695
駅リストは指定終着駅から先の情報は切り捨てられていたが、行先が指定した終着駅になっていなかった
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.