Align SmartInsights query and weekly-volume computation to shared nowMs (Option A)

[9thLevelSoftware]

Motivation

• Prevent subtle timing drift between repository queries and windowed computations by ensuring both use the same effective nowMs anchor.
• Prefer a single snapshot fetch to keep downstream functions (e.g., weekly volume) deterministic relative to the same timestamp.

Description

• Implemented Option A in SmartInsightsTab: capture snapshotNowMs once inside LaunchedEffect(profileId) and assign it to nowMs before doing data fetches, and use snapshotNowMs when calling getSessionSummariesSince(...).
• Updated calls that compute insights to continue using the shared nowMs state so all cards derive their windows from the same anchor.
• Added documentation next to the repository fetch in SmartInsightsTab explaining the Option A rationale and why it avoids query/computation drift.
• Added a doc note above SmartSuggestionsEngine.computeWeeklyVolume(...) explaining that callers should pass the same effective nowMs used for data fetches and referencing the Option A pattern used in SmartInsightsTab.

Testing

• No automated tests were run as part of this change.

---

Codex Task

---

Perform an AI-assisted review on

[gemini-code-assist]

Code Review

This pull request introduces a snapshot-based timestamp approach in the SmartInsightsTab to ensure that data fetching and subsequent computations are aligned to the same anchor time, preventing timing drift. Review feedback suggests improving the documentation by removing transient design terminology like 'Option A' and avoiding layering violations where the domain layer's SmartSuggestionsEngine referenced specific UI components.

[gemini-code-assist]

The documentation here references a specific UI component (SmartInsightsTab) and a transient design term ("Option A"). This creates a layering violation where the domain layer depends on knowledge of the presentation layer. Additionally, mentioning a "28-day fetch" in the context of a weekly volume function (which only needs 7 days) can be confusing for future maintainers. It is better to describe the requirement abstractly.

Suggested change

	* NOTE: Callers should pass the same effective nowMs used to fetch sessions from storage.
	* In SmartInsightsTab we intentionally use Option A (single 28-day fetch + shared nowMs),
	* then derive the weekly window here to keep query and computation aligned.
	* NOTE: Callers should pass the same effective nowMs used to fetch sessions from storage.
	* This ensures that the data query and the windowed computation are aligned to the same
	* anchor timestamp, preventing timing drift.

[gemini-code-assist]

Avoid using transient design terminology like "Option A" in code comments. It is better to describe the implementation strategy directly so it remains clear to future maintainers who may not have context on the original design options discussed during development.

Suggested change

	// Option A: fetch a broad 28-day slice once, then compute every card's time window from
	// the exact same anchor timestamp. This avoids drift where query and computation use
	// slightly different "now" values.
	// Use a single anchor timestamp for both the data fetch and subsequent computations.
	// This prevents timing drift where different cards might otherwise derive windows
	// from slightly different "now" values.

[Copilot]

Pull request overview

This PR aims to prevent timing drift in Smart Insights by ensuring the session query and all subsequent time-window computations use the same nowMs anchor, making insight cards deterministic relative to a single timestamp snapshot.

Changes:

• Capture a single snapshotNowMs in SmartInsightsTab and use it for both nowMs state and the 28-day session query anchor.
• Add explanatory documentation in SmartInsightsTab describing the “Option A” rationale for avoiding query/computation drift.
• Add a doc note in SmartSuggestionsEngine.computeWeeklyVolume(...) advising callers to use the same effective nowMs used for storage fetches.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/screen/SmartInsightsTab.kt	Captures a single nowMs snapshot and uses it consistently for fetching session summaries and computing insight windows.
shared/src/commonMain/kotlin/com/devil/phoenixproject/domain/premium/SmartSuggestionsEngine.kt	Adds KDoc guidance about passing an aligned nowMs to weekly-volume computation.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[kilo-code-bot]

Code Review Summary

Status: 3 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
WARNING	3

Issue Details (click to expand)

WARNING

File	Line	Issue
shared/src/commonMain/kotlin/com/devil/phoenixproject/domain/premium/SmartSuggestionsEngine.kt	46	KDoc references presentation layer (SmartInsightsTab) and transient term "Option A" — creates layering violation and future-maintenance ambiguity
shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/screen/SmartInsightsTab.kt	122	Comment uses transient design terminology "Option A" — should describe implementation directly
shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/screen/SmartInsightsTab.kt	131	withContext(Dispatchers.IO) is redundant (repository already uses IO) and state mutation from IO thread risks race conditions — remove wrapper and assign on main

Other Observations (not in diff)

No issues found in unchanged code.

Files Reviewed (2 files)

• shared/src/commonMain/kotlin/com/devil/phoenixproject/domain/premium/SmartSuggestionsEngine.kt
• shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/screen/SmartInsightsTab.kt

---

_{Reviewed by step-3.5-flash · 80,809 tokens}