Today's 5: sectioned tab — opt-in Suggested + Also done today

.claude/skills/manual-test/SKILL.md

@@ -20,7 +20,7 @@ Generate a checklist of manual tests the user should run for recent changes.
 
 2. Categorize the changes:
    - **Algorithm/logic changes** — things that affect behavior but not UI
-   - **UI changes** — new widgets, layout changes, label changes
+   - **UI changes** — new widgets, layout changes, label changes. If the change affects how a screen *looks* (layout, positioning, spacing, sizing, colors, empty/populated states, overflow), it also needs **design checks** (see step 7), not just functional tests.
    - **Data/DB changes** — schema migrations, new queries
    - **Edge cases** — boundary conditions, empty states, error paths
 
@@ -43,7 +43,8 @@ Generate a checklist of manual tests the user should run for recent changes.
    - **Specify the variant under test.** When a feature has distinct subtypes or modes (e.g. "due by" vs "scheduled on" deadlines, pinned vs unpinned tasks), each test case must state which variant it uses. Don't just say "a task with a deadline" — say "a task with a 'Due by' deadline".
    - **Always state the Inbox toggle state per task.** For EVERY task the user creates where the Inbox toggle is shown — at root level on All Tasks, via the Starred FAB, the **create-from-empty-search** flow (All Tasks search *and* Today's 5 "Pick existing task"), or any future surface that shows it — explicitly say whether Inbox should be **ON** or **OFF** for that specific task (e.g. "tap + FAB, **turn Inbox OFF**, type 'Container'" or "tap + FAB, **keep Inbox ON (default)**"). The toggle defaults ON, so a created-at-root/Starred task with no Inbox instruction is genuinely ambiguous — it changes whether the task lands in the Inbox section or the main list. State it per task in the setup, not just once. Tasks created *inside a parent* (drilled in) don't show the Inbox toggle, so no Inbox mention is needed there.
    - **Inbox-ON tasks are root-level — so never assert "at root" alone.** Inbox tasks *are* at the root, so an expected outcome of "the task is at root" is ambiguous and will read as a bug when the task correctly lands in the Inbox. When a test's expected result is where a created task ends up, distinguish the two root buckets explicitly: an Inbox-ON task lands "**in the Inbox section**"; an Inbox-OFF root task lands "**in the main root list (not Inbox)**". This is the outcome-side twin of the rule above: state the Inbox toggle in the action *and* make the expected result name the correct bucket. (Orthogonality note: pinning to Today's 5 is independent of Inbox membership — a task can be both in the Inbox and pinned, so "in Inbox + pinned" is a valid, non-contradictory outcome.)
-   - **State the expected starting state** before the first test. Tell the user whether their existing app data is fine, or if they need a clean slate. Example: "Your existing tasks/pins won't interfere — these tests create new tasks." or "Clear Today's 5 first (New set → Replace) to start fresh."
+   - **Bulk fixture creation — use "Add multiple" (brain dump), not N separate adds.** When a setup needs several tasks at the same level, tell the user to use the **"Add multiple"** flow (**+ FAB → "Add multiple" → one task name per line → Add**) — do NOT ask them to repeat the single-add flow N times. It's available on the **All Tasks root** add and the **Starred FAB** (the Today's 5 "Create new task" flow hides it). State the **Inbox** toggle **once for the whole batch** (it applies to every line). Fall back to individual single adds only when tasks need **different per-task settings** the batch can't express (mixed Inbox states, per-task pin/deadline, or nesting under different parents).
+   - **State the expected starting state** before the first test — and by default that is a **clean slate via `/cleardb`** (see the `/cleardb` rule below). Tell the user to run `/cleardb` and relaunch `./dev.sh` before Section 1 unless there is an explicit reason to keep existing data (in which case say so, and explain why the existing data is safe — e.g. "Your existing tasks/pins won't interfere — these tests create new tasks.").
    - **Tests that verify a refresh or side-effect MUST spell out the mutation steps.** If a test asks the user to "switch to tab X and confirm Y reflects the latest state", you must include the exact mutation that produces that new state — never assume the user knows what "recently completed", "recently edited", or "the latest" means in context. Bad: "switch to Today → Today's 5 reflects latest state". Good: "On All Tasks, tap any Today's 5 task → tap 'Done today' → switch to Today tab → that task now appears under the Done section". This rule applies to all "X refreshes on entry", "Y reloads on tab switch", "Z reflects external changes" tests.
 
 5. Prioritize tests by risk:
@@ -53,6 +54,13 @@ Generate a checklist of manual tests the user should run for recent changes.
 
 6. Keep the list practical — aim for 5-10 tests, not an exhaustive matrix. Focus on things automated tests can't easily cover (visual correctness, interaction feel, real data scenarios).
 
+7. **Design / visual checks (whenever a UI screen, layout, or styling changed).** Functional test cases verify *behavior* — they do NOT confirm a screen actually *looks* right. When the change touches how a screen looks (new/repositioned/removed widgets, layout, spacing, sizing, alignment, colors, empty vs populated states, overflow handling), add a separate **Design checks** section that walks the user through *seeing* each visual case and asks whether it looks good / matches their expectations. This is a yes/no aesthetic judgement, not a pass/fail action.
+   - **Enumerate the distinct visual states the change can produce — not just the happy one.** Typical axes: **empty vs populated**, **1 item vs many / overflowing**, **short vs long text (truncation / ellipsis)**, **each optional section present vs absent**, **light vs dark theme**, and any state-specific layout the change introduced. One check per distinct visual case, so nothing that only appears in an edge state goes unseen.
+   - **Phrase each as: reach the state → "does it look good / as you expect?"** e.g. "On the Today tab with 0 pins → the 'Nothing pinned yet' hero is centered and the Suggested / Also-done row sits pinned at the bottom → does the empty layout look right to you?"
+   - For each check, spell out exactly how to reach that visual state (reuse the setup discipline from step 4) and **what to look at** (alignment, spacing, no overflow/clipping, position stays consistent across states, icons are self-explanatory, etc.).
+   - The user drives the app on Linux (`./dev.sh`) and judges by eye. **Offer to review it yourself too via `/check-screenshot`** — invite them to share a screenshot of any state they're unsure about, and inspect it against the expectation they stated. Never claim a screen looks correct without actually seeing it.
+   - These are subjective/aesthetic — **never assert the outcome for the user** (per the "no coin flipping" rule). Present the state, say what you intended it to look like, and let them decide.
+
 ## Output Format
 
 ```
@@ -67,6 +75,10 @@ Create [whatever state is needed for the tests below].
 
 ### Regression
 3. Do X → still works as before
+
+### Design checks (only when a screen's look changed)
+D1. Reach [empty state] → [what it should look like] → does it look good / as you expect?
+D2. Reach [populated / overflowing state] → [what to look at] → does it look right?
 ```
 
 ## Rules
@@ -78,7 +90,9 @@ Create [whatever state is needed for the tests below].
 - **Never instruct the user to create state the UI cannot produce.** Some fixtures can't be made through the app: most notably, **the deadline date picker disallows past dates**, so an "overdue" / past-deadline task **cannot** be set up via the UI. Before putting any setup step in the checklist, confirm the UI can actually produce that state (check the picker/dialog constraints in the widget code). When a required fixture is UI-impossible (past-date deadline, a specific `created_at`/`last_worked_at`, a forced sync state, etc.), do NOT write a UI setup step for it. Instead: flag that it needs **direct DB insertion** (`~/.local/share/com.taskroulette.task_roulette/task_roulette.db` via `sqlite3`), and **ask the user for confirmation before writing to the DB**. Note that the running app must be restarted (or the affected tab reloaded) to pick up direct DB changes. If a test exists only to verify the UI-impossible case, still include it — just make its setup the DB-insert path, not a UI path.
 - Don't suggest tests that duplicate what automated tests already cover — check the test files first.
 - **Never include sync/auth tests when running on test data.** The user uses test data on their dev machine — don't suggest signing in, triggering sync, or testing sync status. Only include sync tests if the change is sync-specific AND the user explicitly says they want to test sync.
-- If a change is purely algorithmic with no UI impact, say so and focus tests on observable outcomes.
+- If a change is purely algorithmic with no UI impact, say so and focus tests on observable outcomes — and skip the Design checks section (there's nothing new to look at).
+- **Design checks are their own section and follow the batching rule** — present them one at a time like any other section, after (or alongside) the functional sections, and don't advance until the user has judged every visual case in the batch. They belong ONLY when a screen's look actually changed; a pure logic/data change gets none. When the user is unsure about a state, proactively offer to look via `/check-screenshot` and compare it to what the design was meant to be — do not vouch for a screen you haven't seen.
+- **Prefer a clean-slate `/cleardb` before the round starts — ONCE, not per-section.** Unless there is an explicit reason to keep the existing data, recommend the user run **`/cleardb`** (then relaunch `./dev.sh`) **before Section 1** so the whole round runs against an empty, predictable DB — existing pins/tasks won't interfere with setup or outcomes. Do this **once per round, before the first section only** — do NOT ask for a `/cleardb` before Section 2, Section 3, etc.; later sections build on the same fresh DB plus whatever earlier sections created. `/cleardb` is `disable-model-invocation` and destructive (it backs up first, stops `./dev.sh`, and needs a relaunch), so **you cannot run it yourself** — instruct the user to run it. Reasons to skip/defer it (call them out explicitly): the change is specifically about **existing-data / migration / sync-merge** behavior, the test needs pre-existing fixtures that only live in the current DB, or the user says to keep their data.
 - Mention which platform to test on (Linux via `./dev.sh` unless the change is mobile-specific).
 - **Present tests in batches by section.** Don't dump all test cases at once — show one section at a time (e.g. "Today's 5" tests first, then "All Tasks" after the user reports results). This prevents the list from feeling overwhelming and lets the user focus. **This applies to the caller too** — when presenting the agent's results, you MUST show only the first section and hold back the rest until the user reports results. Never flatten all sections into one list or present them all at once, even as a "summary". **Do NOT move to the next section until the user has reported results for ALL test cases in the current section.** If the user reports on only some tests (e.g. "1. works" but there are 10 tests), re-display the remaining tests from that section — do NOT advance to the next section.
 - **Snackbar undo tests are time-sensitive.** The undo snackbar only lasts 5 seconds. Rules for undo tests:
@@ -87,5 +101,6 @@ Create [whatever state is needed for the tests below].
   - If the undo test requires the same setup as a non-undo test, tell the user to **re-do the setup** (or use a separate task) rather than chaining it after the non-undo test's verification steps.
   - Warn the user about the 5s window, and suggest a manual fallback path (e.g. unarchive + re-add state) if they miss it.
 - The user reports results like "1. works / 2. works". If their list is incomplete (doesn't cover all test cases), don't assume they're skipping the rest. Ask (via AskUserQuestion, with "continue" as the default) whether they want to carry on or skip. If they want to carry on, re-display the remaining test cases so they don't have to scroll up.
+- **Always re-print the FULL text of any test cases you're asking the user to run or re-run — never refer to them by ID/number alone** (e.g. "continue with D5 and D6", "re-check test 3"). Restate the complete item: its setup, action, and expected result, verbatim. This matters most when **intervening work** (bug fixes, screenshot reviews, discussion) has pushed the original checklist several messages up — the user must never have to scroll back to find what a test said. When in doubt, re-print. Cost of re-printing a few lines ≪ cost of the user hunting for them. The same applies when you re-surface remaining tests after a partial report, or resume a section after a fix.
 - **When a test case turns out to be wrong or its precondition was invalidated (e.g. by state a prior test mutated), FIX it — never silently skip it.** The fix is to re-establish the needed state (spin up a fresh dedicated fixture and re-run the test), not to delete the test. Do NOT rationalize dropping coverage by arguing the test is "redundant" or "the same code path" — that is not your call to make unilaterally. If you genuinely believe a test adds no coverage, say so explicitly, explain why, and ask the user whether to drop or keep it. The default is to keep and fix. Skipping a test to save effort is never acceptable; the point of the checklist is coverage, not closing it out fast.
 - **When the bug is in the checklist itself (bad sequencing, stale precondition, wrong UI reference), own it and correct the checklist** — and if the same flaw could recur, update this skill so future checklists avoid it. A wrong test case is a defect in the test, not a reason to abandon the behavior being tested.

.claude/skills/merge-check/SKILL.md

@@ -51,5 +51,7 @@ Both agents MUST run in parallel (launched in a single message with two Agent to
 
 - Do NOT merge if CI has failed.
 - Do NOT skip the review comment check — always wait or ask.
+- **After you push a fix for review comments, WAIT for a re-review — the review is not "done" after one round.** Codex (and Claude, if enabled) **re-reviews each push**, so a fix commit can trigger a **new** round of comments (possibly on the fix itself). Do NOT treat the first round as final and do NOT merge straight after pushing a fix. After committing + pushing fixes, re-run the review-comments poll (Phase 1 step 2 — ~5 min / 10 polls of the three comment endpoints on the **new** head commit for Codex, or wait on `claude-review` if enabled) **and** re-check CI on the new commit.
+- **Do NOT loop on Codex indefinitely — it may never come back "clean."** Codex re-reviews every push and often keeps surfacing fresh nitpicks/P2s round after round, so "keep fixing until no new actionable comments" can run forever. Bound it: poll ~5 min for the new round after a fix push; if a new actionable comment arrives, you may address it **once or twice**, but once rounds show **diminishing returns** (new comments are minor/nitpicky, contentious, or conflict with an explicit user decision), **STOP auto-looping and ask the user what to do.** Offer to let them **review the Codex thread directly in the browser** (the PR's "Codex Review" comments / the linked chatgpt.com/codex settings) and decide whether to address, defer (log a TODO), or merge as-is. Never silently merge over unaddressed actionable comments, and never keep fix-looping without checking in.
 - Do NOT force merge or use `--admin`.
 - Phase 1 MUST run in background so the user can do other work while waiting.

CLAUDE.md

@@ -68,6 +68,8 @@ When mutating a task that is `_currentParent` (e.g. rename, start, unstart), the
 - When writing tests in bulk, use `flutter test --coverage` to find gaps. Parse `coverage/lcov.info` directly (`genhtml` may not be installed).
 - Capture any user-mentioned future work items as todo tasks immediately.
 - **When changing weighting logic**, update `docs/TODAYS_FIVE_ALGORITHM.md` to keep the algorithm doc in sync.
+- **Before any UI design/layout work, ask for ALL the user's requirements up front — never assume or half-read.** UI intent is easy to misread (position, sizing, whether a section should move, what "compact" means, etc.), and guessing wastes iterations. When the user requests a UI change, restate your understanding and ask clarifying questions about layout, positioning, sizing, and behavior *before* editing widgets. Only proceed once the requirements are confirmed.
+- **Reason about BOTH form factors (wide desktop AND narrow phone) for every layout decision.** The app runs on Linux desktop (wide) and Android (narrow). Layout reasoning that holds at one width can break at the other — e.g. "N content-sized pills fit in ~2 rows" is true at desktop width but wraps to N rows on a phone; a row that fits desktop may overflow or need scroll on mobile; FAB overlap, truncation, and columns-per-row all shift with width. Whenever a decision depends on available width (wrapping, columns-per-row, whether something scrolls, truncation, overlap), explicitly check the **phone-width** case too before concluding — never reason only from the desktop window you can see. **Strongly recommend the user run `/debug-build`** to verify on-device whenever a UI change could behave differently on phone vs desktop (wrap/row counts, scroll, overflow, spacing, FAB clearance). Say so proactively.
 - **When changing UI** (adding/removing/modifying dialogs, buttons, toggles, screens, or interaction patterns), update `docs/UI_VIEWS.md` to keep the UI reference in sync, and re-check this at commit time — if any staged files under `lib/screens/` or `lib/widgets/` touched the UI, confirm `docs/UI_VIEWS.md` was updated and remind the user if not. For UI bugs, read `docs/UI_VIEWS.md` first to understand the current UI context before investigating.
 - **When a manual test case is reported as wrong** (user says the test doesn't match actual behavior), immediately update `docs/UI_VIEWS.md` to reflect the correct behavior before correcting the test case. The test was wrong because the docs were incomplete — fix the source of truth first.
 - When setup instructions change (new deps, build steps), ask user if they want to update README.