Fix missing schedule/deadline/norm params in backfill paths#56
Fix missing schedule/deadline/norm params in backfill paths#56
Conversation
… paths Bug fix: 5 of 7 pickWeightedN call sites in todays_five_screen.dart were missing scheduleBoostedIds, deadlineDaysMap, and normData params, silently dropping the 2.5x schedule boost and up to 8x deadline boost during restore/backfill/replacement operations. Only _generateNewSet and _swapTask passed them correctly. Fix: replace standalone getAllLeafTasks() calls with _fetchSelectionContext() in _loadTodaysTasksInner, refreshSnapshots, and _replaceIfNoLongerLeaf, then pass full params to all pickWeightedN calls. Also adds CLAUDE.md guideline requiring all pickWeightedN calls to use _fetchSelectionContext() params. Tests not yet run — WIP commit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 tests verifying that pickWeightedN backfill paths now receive full selection context (scheduleBoostedIds, deadlineDaysMap, normData): - restore backfill in _loadTodaysTasksInner - refreshSnapshots backfill after task becomes non-leaf - pinned-descendant replacement on restore Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3da38a1b7d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!mounted) return; | ||
| final provider = context.read<TaskProvider>(); | ||
| final allLeaves = await provider.getAllLeafTasks(); | ||
| final ctx = await _fetchSelectionContext(); |
There was a problem hiding this comment.
Lazy-load selection context during snapshot refresh
refreshSnapshots() is called on routine Today-tab returns, but this change now eagerly calls _fetchSelectionContext() before any replacement/backfill is detected. That forces extra DB queries (including getNormalizationData, which runs recursive ancestry/count work) even when every task is still a valid leaf and no weighted pick is needed, so normal navigation can incur avoidable latency as task counts grow. Fetch this context only inside the branches that actually call pickWeightedN.
Useful? React with 👍 / 👎.
- Add widget test for _replaceIfNoLongerLeaf path (uncomplete flow) to cover the third backfill fix missing from PR review feedback - Fix "no dialog shown for task without deadline" test hanging: bump pumpAndLoad/pumpAsync rounds to 40 because _fetchSelectionContext now does 5 DB queries instead of 1, needing more FakeAsync pump cycles Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…DB queries Previously, _fetchSelectionContext() was called eagerly at the start of refreshSnapshots(), running 5 DB queries (including recursive norm data) on every Today-tab return even when all tasks were still valid leaves and no weighted pick was needed. Now fetch it lazily only inside the branches that actually call pickWeightedN (pinned-descendant replacement and backfill). Also adds a test verifying refreshSnapshots completes correctly when all tasks remain valid leaves (the common case on routine tab returns). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
pickWeightedNcall sites intodays_five_screen.dartwere missingscheduleBoostedIds,deadlineDaysMap, andnormDataparams, silently dropping the 2.5x schedule boost, up to 8x deadline boost, and normalization during:refreshSnapshotsbackfill (tasks that become non-leaf mid-session)_replaceIfNoLongerLeaf(single task replacement after adding a child)_fetchSelectionContext()to pass full paramsTest plan
flutter analyzepasses🤖 Generated with Claude Code