fix(chat): enable the Recent filter when its window is chosen

[buluoray]

Follow-up to #6909, which merged before its last two advisory verdicts could be answered. Three items from that PR's Design and First Principles CONCERNS shipped to main unaddressed.

Choosing a Recent window did not turn the filter on

#6909 hid the inline window picker while the filter was off, because picking a window stores the window and nothing else — so on an inactive filter the chip went green and the session list did not change. That gate only covered the phone. The wide viewport reaches the same picker through the flyout, so the defect is live on main today for every desktop user.

Fixing it per-modality was the mistake, so picking a window now enables the filter, on every viewport. Intent is decided at the call site rather than by comparing the new window to the stored one: an explicit pick (preset chip, unit change) enables unconditionally, because an identical value does not mean the user did nothing — DEFAULT_RECENT_WINDOW_MS equals the 1 hour preset, so a value-equality gate would swallow exactly the chip a fresh user is most likely to click. The identical-value test survives only on the amount field, which commits on blur as well as Enter and is the one path where "the user did nothing" is real.

The mobile-only picker gate is also gone. It existed because picking a window did not enable the filter, so a visible picker on an inactive filter reported an effect it was not having — this change makes that reason false, and a gate on one viewport is the very per-modality thinking that caused the defect. Removing it also made the mobile test leg real: it used to tap the row to reveal the gated picker, writing the filter flag before the pick, so its assertion passed without the fix.

wide viewport, Recent OFF, click 1 week	mc-session-recent-only
before (main)	null — chip green, list unchanged
after	1 — row checked, count (1)

before	after

The row reads Recent · 7d in both; only the check and the (1) differ, which is the whole point — the state change is invisible in a still of the menu, so the evidence is a driven pick with the persisted flag printed beside it.

Two subtractions First Principles asked for

• NativeSelect.wrapperClassName removed — zero consumers (grep: the defining file only). wrapperStyle is all the seam needs.
• CHEVRON_GUTTER unexported. This one is more than tidying: the test imported it, so paddingInlineEnd === CHEVRON_GUTTER was true by construction and a wrong gutter would have sailed through. The test now states 2.25rem itself — mutating the constant to 2rem reddens three assertions that previously could not fail.

Verification

• 6 mutation-verified guards, all reddening: remove the enable from the picked path; restore the rejected value-equality gate on the shared seam (so the tests can tell the right placement from the wrong one, which the previous suite could not); drop the blur guard; reinstate the mobile gate; change the gutter constant; and the earlier unconditional-enable case.
• New tests loop over both viewports and neither leg touches the filter on its way to the chip, so both actually discriminate (the earlier mobile leg did not — see above).
• tsc clean, eslint 0 errors (9 warnings in ChatSidebar.tsx is the base level).
• Rebased onto current main (0468a40ac), squashed to one commit. The branch was cut at d7b7d65c3 and had drifted 40+ commits, so it was carrying main's own merged work (docs(changelog): backport 0.5.0 insider.4 entries to main #6936's changelog, fix(i18n): interpolate the product name in two catalog values #6932's {{productName}} catalog interpolation) as apparent changes — which is also why I earlier reported src/i18n/productName.test.ts as a "pre-existing base red to ignore". That was wrong: fix(i18n): interpolate the product name in two catalog values #6932 had already fixed it on main, and the branch simply predated the fix. On the new base that test passes and the Changelog History Gate is green.
• The diff is now exactly the 6 files this PR is about. tsc clean, eslint 0 errors, and the full suite is 1,666 files / 26,344 tests with zero failures. jscpd still reports 1 clone; that one does reproduce on pristine current main, verified with this branch's changes absent.

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of f8ad9186dde5e90fb1523395e849d95d45dbe4d4 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] f8ad918

False positive or not applicable? A repository writer can comment:
/ai-review override gpt f8ad9186dde5e90fb1523395e849d95d45dbe4d4: <one-sentence reason>

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of f8ad9186dde5e90fb1523395e849d95d45dbe4d4 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Fixes the root cause at the shared commit seam instead of per-viewport, with the enable/no-enable asymmetry deliberately reasoned and mutation-pinned.

[DESIGN-REVIEWED] f8ad918

[github-actions]

UX Review (Fable 5) — ✅ PASS

UX-level review of f8ad9186dde5e90fb1523395e849d95d45dbe4d4 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The fix is coherent across both modalities: the desktop flyout already showed the picker while the filter was off, and picking now enables the filter through shared commit callbacks, so the mobile inline picker un-gating restores parity rather than diverging. The screenshots confirm the before/after checkmark behavior. One residual gap: commitRecentAmount enables the filter only when the value changed, so Enter on an unchanged custom amount — an explicit apply gesture, unlike blur — still produces silence.

UX-Verdict: PASS

Picking a window now honestly turns the filter on across desktop flyout and phone inline picker — the lying green chip is fixed at the shared commit seam.

Suggestions

• In commitRecentAmount, Enter with an unchanged amount still skips enableFilter('recent') — the guard if (next !== recentWindowMs) protects blur, but Enter is explicit apply intent; pass a flag from the onKeyDown path so Enter always enables, blur stays gated.

[UX-REVIEWED] f8ad918

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed f8ad9186dde5e90fb1523395e849d95d45dbe4d4 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] f8ad918

Verdict parsed from the review's SHA-scoped output markers for commit f8ad9186dde5e90fb1523395e849d95d45dbe4d4.

False positive or not applicable? A repository writer can comment:
/ai-review override fable f8ad9186dde5e90fb1523395e849d95d45dbe4d4: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of f8ad9186dde5e90fb1523395e849d95d45dbe4d4 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks complete. The change survives every lens: the fix sits at the shared commit seam (cause level — the prior fix's per-modality gating was the cause), every other item is a declared deletion, and I found no unfixed siblings (the stale-collapse picker has no separate on/off flag, so it cannot have this defect; enableFilter mirrors the pre-existing disableFilter and has 2 consumers).

First-Principles-Verdict: PASS

A live desktop defect fixed at the one shared commit seam, with two review-requested deletions and no additions riding along.

What this change ships

Intent: make choosing a Recent window actually filter the session list, on every viewport. FIX.

1. Picking a preset chip or unit turns the Recent filter on, all viewports — justified (defect live on main for desktop)
2. Re-committing an unchanged amount on blur does not flip the filter — justified, declared boundary of the fix
3. Phone: window picker visible while Recent is off (gate deleted) — justified deletion; the gate's stated premise is made false by item 1
4. NativeSelect.wrapperClassName removed — justified deletion; zero consumers (grepped wrapperClassName under website/: 0 matches remain)
5. CHEVRON_GUTTER unexported; test states 2.25rem independently — justified deletion of a true-by-construction test
6. Two evidence PNGs under temp-screenshots/recent-window-enables-filter/ — undeclared as repo files, but matches the established convention (2,781 files, per-PR directories)

Depth check: the root cause named — per-modality patching in #6909 — is what items 1 and 3 remove; sibling scan found none (grepped the stale-collapse picker, ChatSidebar.tsx:2915-2978 — it has no separate on/off flag, so picking a value there already takes effect). enableFilter is not a second spelling: toggleFilter (line 2592) would turn an already-on filter off, and the new helper mirrors the pre-existing disableFilter (line 2601).

[FIRST-PRINCIPLES-REVIEWED] f8ad918

[buluoray]

Disposition for the three CONCERNS on ac60d81a3 — head is now 99dcf6380.

Fixed — Design and UX both found a real bug I introduced

if (ms !== recentWindowMs) enableFilter('recent') put a semantic judgment ("did the user intend this?") on a value comparison at a shared seam, where intent is not knowable. Verified their premise exactly:

DEFAULT_RECENT_WINDOW_MS = 60 * 60 * 1000   (recentWindow.ts:14)
RECENT_WINDOW_PRESETS[0] = { label: '1 hour', ms: 60 * 60 * 1000 }

Identical. So a fresh user with the filter off, clicking the highlighted 1 hour chip, hit the equality and got the exact defect this PR exists to fix — on the single chip most likely to be clicked. Both bots reached that independently, and both proposed the same remedy, which is the right one: decide intent at the call site.

An explicit pick (preset chip, unit change) now routes through chooseRecentWindow and enables unconditionally. The identical-value test survives only on the amount field, which commits on blur as well as Enter — the one path where "the user did nothing" is a real case.

The decisive new guard is a mutation at the rejected placement: putting the value gate back on the shared seam reddens enables the filter even when the clicked preset is the one already stored. The previous suite could not tell the two placements apart at all.

Fixed — First Principles: my "both viewports" claim was hollow

Correct, and it is the same true-by-construction flaw this PR removed from the gutter test. The mobile leg tapped the row to reveal the gated picker, and that tap runs toggleFilter('recent'), writing mc-session-recent-only='1' before the chip was clicked — so its final assertion passed with or without the fix. Only the desktop leg guarded anything. That also explains why the earlier mutation round reddened exactly one test rather than two.

Both legs now reach the picker without touching the filter, and each asserts the flag is null on arrival.

Fixed — First Principles subtraction: the mobile picker gate

Taken. Its justifying comment ("Picking a window does not enable it, so an always-visible picker … reports an effect it is not having") is made false by this very diff, and a gate on one viewport is the per-modality thinking the description calls the mistake. Deleting it is also what makes the mobile test leg real. The always-visible picker was checked against the viewport: capture-sidebar-duration-pickers-narrow.mjs still reports every menu surface offLeft: 0, offRight: 0 at 390px in both themes.

Declined, with reasons

• Shrink enableFilter to an inline update. It has 2 consumers after this change (chooseRecentWindow and the amount-commit path), and it is the exact twin of disableFilter sitting immediately above it — inlining one half of an established symmetric pair costs more in readability than the six lines it saves.
• UX: mute the preset chips while the filter is inactive. The chip reports which window is stored, which is true whether or not the filter runs; the row's check reports whether the filter is on. After this change, clicking the highlighted chip is no longer a no-op, so the pair no longer contradicts itself. Advisory suggestion beside a CONCERNS, so optional — happy to take it if you would rather the two states never look alike.

Unchanged

src/i18n/productName.test.ts still fails and jscpd still reports 1 clone; both reproduce on pristine d7b7d65c3 with my commits absent. Everything else: 1,665 files / 26,343 tests green, tsc clean, eslint 0 errors.

[buluoray]

Rebased onto current main (0468a40ac) and squashed to one commit — head is now f8ad9186d. That single action resolved every open item, including one where I was wrong.

First Principles CONCERNS — fixed, and it was one root cause

Correct on all counts. The branch was cut at d7b7d65c3 and main had moved 40+ commits, so the branch diff carried main's own merged work as if it were mine: #6936's ~20 changelog entries and #6932's {{productName}} interpolation across 13 locales. The description said the catalogs were untouched; the diff said otherwise. Rebasing makes the two agree — the diff is now exactly the 6 files this PR is about, with no CHANGELOG.md and no locale files.

Also confirming its sibling count independently: the only other duration picker is stale-collapse, which encodes off as 0 ms, so a pick there always takes effect. Zero unfixed siblings — the fix is general.

A claim of mine that this falsifies

I reported src/i18n/productName.test.ts as a "pre-existing base red, not mine, ignore it", having reproduced it on pristine d7b7d65c3. That reproduction was real but the conclusion was wrong: #6932 had already fixed that test on main, and my branch simply predated the fix. "Reproduces on my base" is not the same as "is broken on main" once the base is stale. On the new base it passes, and the Changelog History Gate — which was red for the same reason, a branch missing main's shipped changelog sections — is green.

The one item that really is main's: jscpd's 1 clone, re-verified on pristine current main with this branch's changes absent.

PR Hygiene — fixed

3 commits against a max of 2. Squashed to one during the rebase.

State on f8ad9186d

• tsc clean, eslint 0 errors (9 warnings in ChatSidebar.tsx is the base level), Changelog History Gate green.
• Full suite 1,666 files / 26,344 tests, zero failures — previously 1 failure.
• capture-sidebar-duration-pickers-narrow.mjs: every open menu surface offLeft: 0, offRight: 0 at 390px in both themes, with the phone picker now always visible.
• 6 mutation-verified guards still reddening, including the decisive one — restoring the rejected value-equality gate on the shared seam reddens the same-value-preset test.