fix(briefing): greet for the actual time of day, not the period#135
Conversation
The morning briefing (last night's sleep + recovery) is shown until 17:00, so the prompt telling the model it's the 'morning' briefing made the AI greet with 'morning' in the afternoon (OpenStrap#134). Pass the reader's real local time of day into the prompt and greet for that; keep the morning/evening period only as the data scope.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughBriefing prompt generation now derives the reader’s local time of day and passes it through system and user prompts, keeping greeting text independent from the briefing period. Tests cover the new arguments, time-based greeting behavior, and metric rendering. ChangesBriefing time awareness
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/ai_briefing_test.dart (1)
124-138: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the generation wiring, not only the pure builders.
This test passes
'afternoon'directly to both prompt builders, so it would still pass ifBriefingEngine.generatestopped deriving or forwardingtod. Extend the generation test with a fixed afternoonnowand assert the captured system and user prompts contain the local-time label.🤖 Prompt for 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. In `@test/ai_briefing_test.dart` around lines 124 - 138, Extend the BriefingEngine.generate test to use a fixed afternoon now value and capture its generated system and user prompts. Assert both prompts contain the corresponding local-time label, verifying generate derives and forwards tod rather than testing only briefingSystemPrompt and buildBriefingUserPrompt directly.
🤖 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 `@lib/ai/briefing_engine.dart`:
- Around line 251-258: Capture a single effective timestamp at the start of the
briefing generation flow, using the provided now value or one DateTime.now()
fallback. Reuse it for todayLabel, partOfDay, collectBriefingInputs, and
generatedAtMs so all generated content shares the same time snapshot.
---
Nitpick comments:
In `@test/ai_briefing_test.dart`:
- Around line 124-138: Extend the BriefingEngine.generate test to use a fixed
afternoon now value and capture its generated system and user prompts. Assert
both prompts contain the corresponding local-time label, verifying generate
derives and forwards tod rather than testing only briefingSystemPrompt and
buildBriefingUserPrompt directly.
🪄 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: 3d8df6a2-8c5d-4db0-b2e8-e07ef7aa8a87
📒 Files selected for processing (2)
lib/ai/briefing_engine.darttest/ai_briefing_test.dart
Reuse a single now for the day label, greeting and generatedAt so a null-now generation can't straddle midnight or a greeting boundary. Addresses CodeRabbit review on OpenStrap#135.
Fixes #134 (reported by @davidfleischmann).
Problem
The AI briefing greeted with "morning" in the afternoon. The
morningbriefing period (last night's sleep + recovery) is shown right up until 17:00 by design, and the prompt hard-coded that framing —briefingSystemPromptsaid "you write the morning health briefing … this morning's recovery" and the user prompt opened "Morning briefing for …". So a reader opening it at 3pm was handed "morning" and the model dutifully greeted with it.Fix
Separate the greeting (which should follow the clock) from the period (a data-scope concept).
generate()already has the current time, so:partOfDay(now)→morning/afternoon/evening/night.The
morning/eveningperiod still drives which data the briefing summarises — only the greeting now tracks the clock.Tests
test/ai_briefing_test.dart:partOfDayboundaries, and that a morning-period prompt built for an afternoon reader says "currently afternoon" and never "morning".Verified by inspection + unit tests (no SDK here to run
flutter test); worth a quick on-device look that an afternoon briefing now reads right.Closes #134
Summary by CodeRabbit
New Features
Bug Fixes
Tests