Conversation
… Calendar integration
… umbrella decision
Preview URLs
|
There was a problem hiding this comment.
Pull request overview
バックエンドに「朝ブリーフィング」関連の統合API(Calendar / Transit / Weather)を追加し、フロントエンドに各API疎通確認用のテストページとAPIクライアントを追加するPRです。
Changes:
- Better Auth セッションを前提に、Calendar(今日の予定)/ Transit(Routes API)/ Morning Briefing(統合)APIを追加
- Open‑Meteo を用いたジオコーディング+降水確率/雨量による「傘判定」ロジックを追加
- フロントエンドに API 呼び出しヘルパーと、Chakra UI のAPI接続テストページを追加
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/lib/backend-api.ts | backend向け fetch ヘルパー(Calendar/Transit/Briefing)を追加 |
| frontend/src/app/api-test/page.tsx | API疎通確認用のテストページを追加 |
| backend/src/types/env.d.ts | Routes APIキー・AI・天気関連のENV型定義を追加 |
| backend/src/routes/transit-routes.ts | POST /transit/directions ルートを追加 |
| backend/src/routes/calendar-routes.ts | GET /calendar/today ルートを追加 |
| backend/src/routes/briefing-routes.ts | POST /briefing/morning ルートを追加 |
| backend/src/routes/root-routes.ts | ルート一覧(endpoints)に新エンドポイントを追記 |
| backend/src/features/transit/transit.types.ts | Transit検索のリクエスト/レスポンス型を追加 |
| backend/src/features/transit/transit.service.ts | Google Routes API v2 での経路検索実装を追加 |
| backend/src/features/weather/weather.types.ts | 天気/傘判定の返却型を追加 |
| backend/src/features/weather/weather.service.ts | Open‑Meteo による天気取得・傘判定を追加 |
| backend/src/features/morning-briefing/morning-briefing.types.ts | 朝ブリーフィングのリクエスト/レスポンス型を追加 |
| backend/src/features/morning-briefing/morning-briefing.service.ts | Calendar/Transit/Weatherを束ねた朝ブリーフィング生成を追加 |
| backend/src/features/google-calendar/google-calendar.types.ts | Google Calendarの簡略イベント型を追加 |
| backend/src/features/google-calendar/google-calendar.service.ts | Better Authのトークンを使った今日の予定取得を追加 |
| backend/src/app.ts | 新規ルート(calendar/transit/briefing)の登録を追加 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| return defaultLocalApiBaseUrl; |
There was a problem hiding this comment.
resolveApiBaseUrl() always falls back to http://localhost:8787 even when running on a non-local hostname and NEXT_PUBLIC_API_BASE_URL is unset. In production this will silently call a nonexistent localhost backend instead of failing fast; consider matching the existing pattern in auth-api.ts/task-workflow-api.ts (infer only for localhost, otherwise throw with a clear message).
| } | |
| return defaultLocalApiBaseUrl; | |
| throw new Error( | |
| `NEXT_PUBLIC_API_BASE_URL is not set and cannot be inferred for hostname "${hostname}". ` + | |
| "Please set NEXT_PUBLIC_API_BASE_URL in your environment.", | |
| ); | |
| } | |
| throw new Error( | |
| "NEXT_PUBLIC_API_BASE_URL is not set and cannot be inferred in this environment. " + | |
| "Please set NEXT_PUBLIC_API_BASE_URL in your environment.", | |
| ); |
|
|
||
| /** | ||
| * Lightweight helpers for calling backend API endpoints. | ||
| * Reuses the same base-URL resolution logic as auth-api.ts. |
There was a problem hiding this comment.
The header comment says this file “Reuses the same base-URL resolution logic as auth-api.ts”, but the implementation differs (notably: this file never throws when NEXT_PUBLIC_API_BASE_URL is missing on non-local hosts). Please align the comment with reality or refactor to share the same resolver to avoid future drift.
| * Reuses the same base-URL resolution logic as auth-api.ts. | |
| * Resolves the backend API base URL, defaulting to a local development server | |
| * when running on localhost. |
| // 3️⃣ Weather — check at the departure location around the leave-by time | ||
| let weather: WeatherInfo | null = null; | ||
| try { | ||
| const weatherLocation = urgent?.destination ?? req.currentLocation; |
There was a problem hiding this comment.
weatherLocation is set to urgent?.destination (event location) when available, but the response type comment says weather is for the departure location around leave-by time. If the intent is “weather at current location when leaving”, this should use req.currentLocation; otherwise update the type/docs to avoid confusing API consumers.
| const weatherLocation = urgent?.destination ?? req.currentLocation; | |
| const weatherLocation = req.currentLocation ?? urgent?.destination; |
| * Model: | ||
| * slackMinutes = (event start minutes) − (now minutes) − transitMinutes − prepMinutes | ||
| * | ||
| * slack >= 15 min → 0% (comfortable) | ||
| * slack <= −10 min → 100% (basically late) | ||
| * In between → linear 0–100 + a flat 10% transit-delay buffer | ||
| * |
There was a problem hiding this comment.
The computeLateRisk comment defines slackMinutes as including both transitMinutes and prepMinutes, but slackMinutes is currently computed as leaveByMin - nowMinutes (prep time not included). Please either update the comment/model description or incorporate prepMinutes into the slack/risk calculation so the docs match the behavior.
| * @param location - Place name or address (used for geocoding). | ||
| * @param targetHHmm - "HH:mm" in JST to look up (e.g. departure time). Falls back to current hour. | ||
| * @param env - Worker env (optional overrides for lat/lon/thresholds). |
There was a problem hiding this comment.
The JSDoc says targetHHmm “Falls back to current hour”, but getWeather()/pickSlot() currently do not implement any fallback if targetHHmm is empty/invalid (it will just return null and report failure). Either implement the fallback (e.g. default to current JST hour) or adjust the docs to match the behavior.
| // 2️⃣ Transit + risk for each event with a location (in parallel) | ||
| const apiKey = env.GOOGLE_MAPS_API_KEY; | ||
| const briefings: EventBriefing[] = apiKey | ||
| ? await Promise.all( | ||
| withLocation.map((event) => | ||
| buildEventBriefing(apiKey, req.currentLocation, event, prepMinutes, nowMinutes), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Promise.all(withLocation.map(...getTransitDirections...)) issues one paid Google Routes API request per calendar event with a location, with no cap/concurrency limit. If a user has many events, this can increase latency and cost significantly; consider limiting to the next N events, only computing for earliestEvent, or adding a simple concurrency limiter.
| type TestId = | ||
| | "session" | ||
| | "calendar" | ||
| | "transit" | ||
| | "weather" | ||
| | "briefing"; | ||
|
|
There was a problem hiding this comment.
TestId/initialResults include a weather entry, but there is no runnable weather test (the card only shows explanatory text). This causes the summary badge to always show weather: idle, which is misleading; either remove weather from the tracked results or derive its status from the briefing response.
| "routes.legs.steps.localizedValues", | ||
| "routes.legs.departureTime", | ||
| "routes.legs.arrivalTime", |
There was a problem hiding this comment.
FIELD_MASK does not include several fields that the code later reads (e.g. routes.legs.localizedValues.* and routes.legs.steps.staticDuration). With a field mask, omitted fields will be absent, so the later fallbacks will never work and step durations may be wrong. Add the needed fields to the mask or remove the unused accesses/fallbacks.
| "routes.legs.steps.localizedValues", | |
| "routes.legs.departureTime", | |
| "routes.legs.arrivalTime", | |
| "routes.legs.steps.localizedValues", | |
| "routes.legs.steps.staticDuration", | |
| "routes.legs.departureTime", | |
| "routes.legs.arrivalTime", | |
| "routes.legs.localizedValues", |
| durationMinutes: Math.ceil( | ||
| parseDurationSeconds(step.staticDuration ?? leg.duration) / 60, | ||
| ), |
There was a problem hiding this comment.
Per-step durationMinutes falls back to leg.duration when step.staticDuration is missing, which would assign the whole-leg duration to each step. Once the field mask is fixed, consider falling back to a step-level duration field (or 0) rather than the entire leg duration.
| const probThreshold = env?.WEATHER_UMBRELLA_PROB_THRESHOLD | ||
| ? Number(env.WEATHER_UMBRELLA_PROB_THRESHOLD) | ||
| : DEFAULT_PROB_THRESHOLD; | ||
| const mmThreshold = env?.WEATHER_UMBRELLA_MM_THRESHOLD | ||
| ? Number(env.WEATHER_UMBRELLA_MM_THRESHOLD) | ||
| : DEFAULT_MM_THRESHOLD; | ||
|
|
||
| // 1) Geocode the location | ||
| const geo = await geocode(location); | ||
| const lat = geo?.lat ?? (env?.WEATHER_DEFAULT_LAT ? Number(env.WEATHER_DEFAULT_LAT) : FALLBACK_LAT); | ||
| const lon = geo?.lon ?? (env?.WEATHER_DEFAULT_LON ? Number(env.WEATHER_DEFAULT_LON) : FALLBACK_LON); |
There was a problem hiding this comment.
ENV threshold overrides are parsed with Number(...) without validating the result. If an override is set to a non-numeric value, probThreshold/mmThreshold become NaN and the umbrella rules will never trigger. Consider using Number.isFinite() and falling back to the defaults when parsing fails.
| const probThreshold = env?.WEATHER_UMBRELLA_PROB_THRESHOLD | |
| ? Number(env.WEATHER_UMBRELLA_PROB_THRESHOLD) | |
| : DEFAULT_PROB_THRESHOLD; | |
| const mmThreshold = env?.WEATHER_UMBRELLA_MM_THRESHOLD | |
| ? Number(env.WEATHER_UMBRELLA_MM_THRESHOLD) | |
| : DEFAULT_MM_THRESHOLD; | |
| // 1) Geocode the location | |
| const geo = await geocode(location); | |
| const lat = geo?.lat ?? (env?.WEATHER_DEFAULT_LAT ? Number(env.WEATHER_DEFAULT_LAT) : FALLBACK_LAT); | |
| const lon = geo?.lon ?? (env?.WEATHER_DEFAULT_LON ? Number(env.WEATHER_DEFAULT_LON) : FALLBACK_LON); | |
| const probEnv = env?.WEATHER_UMBRELLA_PROB_THRESHOLD; | |
| const probThreshold = | |
| probEnv && Number.isFinite(Number(probEnv)) | |
| ? Number(probEnv) | |
| : DEFAULT_PROB_THRESHOLD; | |
| const mmEnv = env?.WEATHER_UMBRELLA_MM_THRESHOLD; | |
| const mmThreshold = | |
| mmEnv && Number.isFinite(Number(mmEnv)) | |
| ? Number(mmEnv) | |
| : DEFAULT_MM_THRESHOLD; | |
| // 1) Geocode the location | |
| const geo = await geocode(location); | |
| const envLat = env?.WEATHER_DEFAULT_LAT; | |
| const fallbackLat = | |
| envLat && Number.isFinite(Number(envLat)) | |
| ? Number(envLat) | |
| : FALLBACK_LAT; | |
| const envLon = env?.WEATHER_DEFAULT_LON; | |
| const fallbackLon = | |
| envLon && Number.isFinite(Number(envLon)) | |
| ? Number(envLon) | |
| : FALLBACK_LON; | |
| const lat = geo?.lat ?? fallbackLat; | |
| const lon = geo?.lon ?? fallbackLon; |
…ment in getTodayEvents function
…nd improve styling
…efing and update data structures
概要
変更内容
Google Calendar連携 (google-calendar.service.ts): Better AuthのOAuthトークンを使って今日の予定を取得する GET /calendar/today エンドポイント追加
乗換検索 (transit.service.ts): Google Routes API (v2) による経路検索 POST /transit/directions エンドポイント追加
天気・傘判定 (weather.service.ts): Open-Meteo(無料・APIキー不要)でジオコーディング→時間帯別降水確率→傘が必要か判定
朝ブリーフィング (morning-briefing.service.ts): 上記3つを統合した POST /briefing/morning エンドポイント追加。遅刻リスク計算(余裕15分で0%、-10分で100%、乗換遅延バッファ+10%)
環境変数 (env.d.ts): GOOGLE_MAPS_API_KEY, AI, 天気関連の閾値オプション追加
APIクライアント (backend-api.ts): Calendar/Transit/Briefing の3つのfetch関数
API接続テストページ (api-test/page.tsx): 各APIを個別にテストできるChakra UIページ。ステータスバッジ・応答時間・JSONレスポンス表示付き
動作確認方法
セッション確認: ユーザー情報が表示される
Google Calendar: 今日の予定配列が返る(GCPでCalendar API有効化が必要)
乗換検索: 経路情報が返る(GCPでRoutes API有効化が必要)
朝ブリーフィング: カレンダー+乗換+天気の統合結果が返る
チェックリスト
関連 Issue