Skip to content

feat(mobile): search input also filters the Active now tray - #4823

Merged
iscekic merged 5 commits into
mainfrom
active-search-08a5
Jul 28, 2026
Merged

iscekic merged 5 commits into
mainfrom
active-search-08a5

Conversation

@iscekic

@iscekic iscekic commented Jul 28, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

What: The Agents session-list search input now also filters the pinned "Active now" tray, client-side, with the same semantics as the server-side history search: a case-insensitive literal substring match on the session's title OR id, in conjunction with the existing platform/project filters.

Why: The tray previously ignored the search text by deliberate design (PR #4626). A user explicitly asked to reverse that decision: when they search, they expect every session list on the screen — including the pinned active tray — to narrow. The code's own comments anticipated this reversal.

How:

  • selectPinnedActiveSessions gains a required searchQuery: string param. The needle is trimmed/lowercased once, then applied as the first check inside the existing filter (String.includes on title/id — literal substring, LIKE wildcards match literally, mirroring the server's position() search). The platform/project logic below is unchanged.
  • AgentSessionListScreen passes effectiveSearchQuery (the same committed, debounced value the history data source consumes), so the tray and history narrow together.
  • selectSessionListBodyModel gains one guard: while searching with an empty history but a populated tray, it returns render-list instead of filtered-empty — tray matches are matches, so the body never claims "No sessions match" beneath a populated tray (that copy would be factually wrong on the feature's primary happy path: searching an active session's own title empties history by construction, since its stored row is excluded). The query-error branch (Retry CTA) and the filter-only branch (pre-existing deliberate decision) are unchanged.
  • Comments/tests encoding the old "search is intentionally NOT a narrowing input" decision are updated (helper JSDoc, screen comment, body-model comments, both test suites — the flipped @ts-expect-error assertion is replaced by 8 positive search-semantics tests).

Verification

  • E2E on iOS simulator (iPhone 17 Pro Max, iOS 26.5, Maestro) against this worktree's local stack — VERIFICATION PASSED. (round r3; run on merged head dd976cf01, and the later main-merge to the current head touched no mobile or owned files — clean merge, zero path overlap). Data: live active session "Bicycle pump haiku generation" plus stored "Zucchini haiku generation" and "New session". All five flows passed:

    1. Query bicycle (title substring, lowercase vs a capitalized title) → the tray row stays, ACTIVE NOW visible, and no "No sessions match" copy renders beneath the tray (tray-only-match state, AC1+AC3).
    2. Gibberish query qxzjkv → the tray hides; "No sessions match / Try a different search term." with a "Clear search" CTA appears (AC2, empty state).
    3. "Clear search" CTA (and the in-field X) → the full tray and full history return (AC4).
    4. Query zucchini → the stored list narrows to "Zucchini haiku generation" only, "New session" hidden, tray hides correctly — history search unchanged (AC5, regression).
    5. Session-id substring 0570dbf84 → the tray row stays (id-matching half of AC1).

    Feature-state matrix: happy PASS, tray-only-match PASS, empty PASS; retryable-unhappy skipped (pre-existing machinery untouched by this change; a deterministic search failure would require fault injection); non-retryable-unhappy N/A (structurally impossible — a pure client-side filter over an already-loaded list has no terminal failure mode). Post-run the worktree was byte-identical to baseline and the simulator/slot were released.

  • Automated (rerun on the final merged head): pnpm format, pnpm typecheck, pnpm lint (0 warnings / 0 errors), pnpm check:unused, and the mobile vitest suite (277 files / 2346 tests, incl. the new search rules helper tests and the new body-model guard tests) all pass in apps/mobile.

Visual Changes

Before After
The search input narrows only stored history; the pinned "Active now" tray ignores it and keeps showing every active session. The search input also narrows the tray: only active sessions whose title or id match stay pinned; with no match the tray hides; when only the tray matches, no contradictory "No sessions match" copy appears beneath it.

No visual-design changes — tray, rows, and empty states reuse existing components; only which sessions appear changes. Simulator screenshots from the passing E2E run (tray before search, tray with a matching query, no-match empty state, restored tray) were captured and verified during verification.

Reviewer Notes

  • Focus areas: the matching semantics in selectPinnedActiveSessions (parity with the server's case-insensitive substring search), and the single new body-model guard (isSearching && hasPinnedActive → render-list).
  • Accepted residuals (deliberate, from the approved plan): on query refinement there is a transient (one search round-trip) window where the tray filters by the new needle while history still shows the previous results under keepPreviousData — consistent with the history list's own stale-while-refetching design; true displayed-row lockstep was rejected as unjustified state for a sub-second window. Unicode case-folding beyond ASCII parity with Postgres lower() is out of scope.
  • The filter-only (platform/project, no text) body-model branch is deliberately untouched — its "populated tray does not change the decision" behavior is a separate pre-existing product decision outside this change.
  • Pre-existing issue surfaced during review (not introduced here, suggested follow-up): the tray's displayed title comes from the live heartbeat payload and cliSessionsV2.rename updates only the stored row — so renaming an active session never updates the title shown in the tray while it stays active. Consequence under this PR's tray filtering: searching the new name matches nothing visible (the stored row is excluded from history while active; the tray still shows the old heartbeat title). The root fix is backend: propagate the renamed title into the live session state, or overlay the stored title in the activeSessions.list enrichment. Full triage in the PR comments.

@iscekic iscekic self-assigned this Jul 28, 2026
@kilo-code-bot

kilo-code-bot Bot commented Jul 28, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

Since the last review, the only change is a new workflow learnings doc (no functional code changes); the previously reviewed search-narrowing logic in the pinned tray and body-model guard remains correct with no high-confidence issues found in changed lines.

Files Reviewed (6 files)
  • .kilo_workflow/learnings/mobile-simulator-claim-killed-mid-boot.md (new, non-functional documentation)
  • apps/mobile/src/components/agents/session-list-body-model.ts
  • apps/mobile/src/components/agents/session-list-body-model.test.ts
  • apps/mobile/src/components/agents/session-list-helpers.ts
  • apps/mobile/src/components/agents/session-list-helpers.test.ts
  • apps/mobile/src/components/agents/session-list-screen.tsx
Previous Review Summary (commit a1a31c1)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit a1a31c1)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Reviewed the client-side search-narrowing changes to the pinned "Active now" tray and the body-model guard; the substring-match semantics, conjunction with platform/project filters, and the new body-model branch are correctly implemented and covered by matching tests, with no high-confidence bugs found in changed lines.

Files Reviewed (5 files)
  • apps/mobile/src/components/agents/session-list-body-model.ts
  • apps/mobile/src/components/agents/session-list-body-model.test.ts
  • apps/mobile/src/components/agents/session-list-helpers.ts
  • apps/mobile/src/components/agents/session-list-helpers.test.ts
  • apps/mobile/src/components/agents/session-list-screen.tsx

Reviewed by claude-sonnet-5 · Input: 20 · Output: 4.3K · Cached: 369.7K

Review guidance: REVIEW.md from base branch main

@iscekic

iscekic commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

(bot) @kilocode-bot please review

@kilo-code-bot kilo-code-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — tray search filtering

Reviewed the five changed mobile files plus the surrounding consumers (session-list-content.tsx, session-list-content-surface.ts, use-session-search-input.ts, use-agent-sessions.ts) and the server search (cliSessionsV2.search). The core implementation is clean and the wiring is correct; below are three substantive findings and some coverage nits.

What checks out

  • selectPinnedActiveSessions — needle is trimmed/lowercased once outside the predicate, includes on title/id is a literal substring test (so %, _, \ match literally, matching the router's position() approach), the early return false sits before the platform/project logic so the conjunction semantics are unchanged, and ActiveSession.title is z.string() (non-nullable) so .toLowerCase() can't blow up on a missing title. Empty/whitespace-only ⇒ needle.length === 0 ⇒ no narrowing. Making the param required is the right call: it forces every call site through typecheck, which is a stronger guarantee than the @ts-expect-error probe it replaces.
  • Screen wiring — effectiveSearchQuery is exactly the value the sections memo already consumes, so tray and history read the same committed query; the useMemo dep list (activeSessions, effectiveSearchQuery, platformFilter, projectFilter) is complete.
  • hasPinnedActive is now search-dependent, so I checked the downstream consumers for fallout and found none: selectSessionListContentSurface's full-screen-error branch is gated on !hasAnySessions, and hasAnySessions is still computed from the unfiltered activeSessions/storedSessions, so a search failure whose text filters the tray to empty can't blank the screen. showInlineError can now go false in that state, but the error signal is still carried by query-error-empty, so nothing is lost.
  • Body-model guard — ordered after the isError branch (Retry/clear-search preserved on failure), returns the same shape as the other render-list sites, and session-list-content.tsx already suppresses ListEmptyComponent for render-list, so the tray-only state renders the tray with no contradictory copy and no stray footer. "Clear search" is still reachable via the in-field X.

1. The trim is a divergence from the server, not parity (minor, but the JSDoc claims parity)

buildAgentSessionSearchInput passes search_string: options.searchQuery verbatim, and the router does search_string.toLowerCase() — no trim on either side. The helper trims. Consequences:

  • " login" → tray matches a title starting with Login…; the server search does not. The tray is strictly more permissive than history for any query with surrounding whitespace (easy to produce on mobile: leading space from the space bar, trailing space from autocorrect).
  • Whitespace-only (" ") → isSearching is true (searchQuery.length > 0, untrimmed), so the server runs a literal three-space substring search (probably empty history) while the tray is not narrowed at all — and the new guard then suppresses the "No sessions match" copy, so the user sees the full tray with no feedback that a query is active.

Neither is a hard bug, but the cheapest fix is one trim at the commit boundary (the debounce controller, or buildAgentSessionSearchInput) so isSearching, the server needle, and the tray needle all derive from the same trimmed string; keep the helper's trim as defense in depth. Otherwise I'd soften the JSDoc's "same semantics as the server-side history search" to call out the trim.

2. A search-matching active session can now become invisible (real, narrow regression)

sections is built from excludeActiveFromGroups(storedGroups, activeSessionIds), and activeSessionIds covers all active sessions regardless of the tray filter. So history hides an active session by id, while the tray now decides by the live heartbeat title. Those two titles can drift: activeSessions.list takes the title straight from the session-ingest worker and enriches only platform/timestamps, while renameSession writes the DB row (and the stored cache) — the live tray title is never updated. Sequence:

  1. Session is active; user renames it to "Payments refactor" from mobile/web. Tray keeps showing the old heartbeat title.
  2. User searches payments. The server matches the stored row → it is dropped from history because its id is active. The tray drops it because the live title doesn't contain the needle.
  3. Net result: a session that the search matched renders nowhere — and with an empty tray the body falls to filtered-empty, so the user is told "No sessions match".

Before this PR the tray always showed it, so the session was at least reachable. Same shape applies whenever the live title is empty/placeholder while the stored title carries the searchable text (RemoteSessionRow renders "Untitled session" for title === '', which is itself unsearchable — consistent with the server's COALESCE(title, ''), but worth pinning in a test). Two ways out, both small: exclude only the pinned ids from history (excludeActiveFromGroups(storedGroups, new Set(pinnedActive.map(s => s.id)))), or match the tray against the stored title as well when one is known for that id (the screen already builds organizationIdBySessionId from storedSessions, so the lookup is free). The first is a one-liner and also self-heals any future tray-narrowing rule.

3. Guard asymmetry vs the filter-only branch

The PR is explicit that filter-only narrowing keeps filtered-empty under a populated tray, and I agree that's out of scope for the behavior change — but the justification in the new comment ("tray matches are matches, so the body never claims 'No sessions match' beneath them") applies verbatim to platform/project filters, since selectPinnedActiveSessions applies those filters to the tray too. The tray under a filter is, by construction, a set of matches. So the codebase now encodes two contradictory answers to the same question, and the comment explaining the search side reads as an argument for changing the filter side. Fine to leave, but I'd track it as a follow-up rather than let the comment imply the filter behavior is principled.

Test coverage

The 8 new helper tests are well chosen — in particular the 100% case genuinely discriminates literal-vs-wildcard ('1000 done' would match under LIKE semantics). Gaps:

  • No test for whitespace around a real term (' login '), which is the trim behavior that actually diverges from the server (finding 1). Whitespace-only is covered; the interesting case isn't.
  • No test for title: '' (tray shows "Untitled session"; searching untitled must not match).
  • No test that the tray consumes effectiveSearchQuery rather than the raw searchQuery. The "unfiltered tray while the first fetch is pending" semantic is only covered by the E2E run, and it's exactly the kind of one-token detail a future refactor can silently flip. A tiny derived-value helper (selectEffectiveSearchQuery(isSearching, isPending, searchQuery)) would make it unit-assertable, if you think it's worth the indirection.
  • Body-model tests cover the new guard plus both non-firing neighbours (filter-only, query-error) — good; nothing missing there.

Nits

  • The unrelated .kilo_workflow/learnings/… doc rides along in this PR; history also carries two main merges. Not blocking if that matches your workflow, but a rebase would keep the diff to the feature.
  • session-list-helpers.test.ts line ~117 comment now runs past the file's wrapping width used by its neighbours — cosmetic.

Summary: finding 2 is the only one I'd want addressed before merge (one-line fix available); finding 1 is a small semantics/doc mismatch; finding 3 is a follow-up. Everything else looks solid.

@kilo-code-bot

kilo-code-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

The review's already posted on the PR. Key takeaway to flag to the author:

  1. Trim divergence — history search uses the untrimmed search_string, but the tray helper trims first. " login" matches tray but not history; whitespace-only query leaves tray unnarrowed while empty-state copy is suppressed. Suggest trimming once at the commit boundary for parity.
  2. Real regression worth fixing pre-merge — excludeActiveFromGroups drops all active ids from history, while the tray decides visibility off the live heartbeat title (which renameSession doesn't update). A renamed active session matched server-side can vanish from both history and tray, with body copy claiming "No sessions match." Fix: exclude only pinned ids from history, not all active ids.
  3. Minor: the new "don't show empty copy under a populated tray" guard's rationale applies equally to the pre-existing filter-only branch — worth a follow-up.
  4. Test gaps: no case for surrounding whitespace, no title: '', no assertion that tray uses effectiveSearchQuery rather than raw query.

Nothing further to do unless you want me to spawn a follow-up Cloud Agent session to actually fix #1/#2.

@iscekic

iscekic commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

(bot) Triage of the four takeaways, each verified against the code on the current head (01b94342):

  1. Trim divergence — not reachable; the suggested fix already exists. The search controller trims at the commit boundary exactly as suggested: commitSearchQuery(text.trim()) (apps/mobile/src/components/agents/session-search-state.ts:93); both clear paths commit ''. The committed query is the single input to both consumers — the server fetch (useAgentSessionSearch({ searchQuery, … })) and the tray (searchQuery: effectiveSearchQuery, derived from that same committed state, session-list-screen.tsx:177,190). No untrimmed value reaches either side, so " login" cannot match the tray but not history. A whitespace-only query commits as '', so isSearching is false (session-list-screen.tsx:89) — the empty-state suppression divergence cannot occur. The helper's own trim is defensive-only, by design.

  2. Renamed active session vanishing — scenario verified real, but the root cause is pre-existing and out of this PR's scope. Both premises confirmed: the tray title comes from the live heartbeat payload (activeSessions.list → session-ingest /api/sessions/active; the router's DB enrichment overlays only status/platform/timestamps — never title), and cliSessionsV2.rename writes only cli_sessions_v2.title (cli-sessions-v2-router.ts:1311). So a mobile rename never updates the title the tray displays while the session is active — that display staleness exists today, with or without search. This PR's filter matches the corpus the UI actually displays: the session's visible title genuinely doesn't match the renamed string. The suggested fix (exclude only pinned ids from history) would change the deliberate history-exclusivity invariant (active sessions never appear in history — a recorded non-goal of this change) and would surface non-pinned active sessions in history — a product decision beyond this brief. The root fix is propagating the renamed title into the live session state, or overlaying the stored title in the active-sessions enrichment — backend follow-up, now flagged in the PR's Reviewer Notes.

  3. Filter-only branch — deliberate and recorded. The new guard is gated on isSearching on purpose: the filter-only branch's "populated tray does not change the decision" behavior is a separate pre-existing product decision this brief does not reverse (plan non-goal; a body-model test pins it). Agreed it is follow-up material, not this PR.

  4. Test gaps — mostly covered or unreachable. A whitespace-only query is tested ("whitespace-only query performs no narrowing", session-list-helpers.test.ts). Surrounding-whitespace input cannot reach the helper from the UI (commits are trimmed — see point 1). Empty-title behavior is pinned by the id-fallback test ("matches a session id substring, case-insensitively" — a non-matching title falls through to id). The tray's query source is one reviewed wiring line (searchQuery: effectiveSearchQuery, session-list-screen.tsx:190) implementing the deliberate pending-window design; no screen-render test harness exists in the owned paths, and the behavior is E2E-verified on a simulator.

No code changes result from this round. The posted summary (No Issues Found | Recommendation: Merge, covering all 6 PR files including the learnings doc) reviewed exactly the diff still on this PR — the head moves since (99bdc009 → 01b94342) are merges of main that change no PR file.

@iscekic

iscekic commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

(bot) @kilocode-bot please review the current head 01b9434 — since your last summary the branch only merged latest main (the PR diff is unchanged: the same 6 files you approved).

@kilo-code-bot

kilo-code-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The PR diff is unchanged since the last review. Verified at head 01b94342: git patch-id --stable of the diff vs base is byte-identical to the previously reviewed diff (2b49c732… in both cases), the same 6 files are touched, and the blob hashes of all 6 files are identical between the previously reviewed head (99bdc009) and 01b94342. The only head movement is a merge of origin/main; the merge base is now main's tip (e5fb4f27), so the branch is fully up to date and the merge introduced no changes to any PR file (no evil-merge content — git diff main..01b94342 lists exactly the 6 files).

The single main commit absorbed since the last review (e5fb4f27 — fix(workflow): stop e2e-slot mangling the stack session name) touches only .kilo_workflow/e2e-slot.sh; nothing under apps/mobile changed on main, so there is no semantic drift in the dependencies of the reviewed code (selectPinnedActiveSessions, selectSessionListBodyModel, the screen wiring) and no merge-introduced regression surface. Re-read the functional hunks at this head to confirm: the tray narrowing (case-insensitive substring on title OR id, trimmed needle, conjunction with platform/project filters) and the isSearching && hasPinnedActive → render-list guard are unchanged and remain correct. CI is green at this head (lint, typecheck, test, format-check, drizzle-check, CodeQL); the BLOCKED merge state reflects the pending approval requirement, not a conflict — mergeable: MERGEABLE.

Prior-round discussion items (trim divergence, renamed-active-session staleness, filter-only branch symmetry, test gaps) were triaged by the author and are unaffected by the merge; they remain either unreachable given the trim-at-commit-boundary design or pre-existing/out-of-scope follow-ups. No new issues found.

Files Reviewed (6 files — identical to previous review)
  • .kilo_workflow/learnings/mobile-simulator-claim-killed-mid-boot.md (docs only, non-functional)
  • apps/mobile/src/components/agents/session-list-body-model.ts
  • apps/mobile/src/components/agents/session-list-body-model.test.ts
  • apps/mobile/src/components/agents/session-list-helpers.ts
  • apps/mobile/src/components/agents/session-list-helpers.test.ts
  • apps/mobile/src/components/agents/session-list-screen.tsx

@kilo-code-bot

kilo-code-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review already posted: no issues found, diff unchanged from the previously approved head, verified byte-identical via patch-id and blob hashes. Recommendation stands: Merge. No further action needed.

@iscekic
iscekic requested a review from pandemicsyn July 28, 2026 17:50
@iscekic iscekic added the human-ready The PR is ready for human review. label Jul 28, 2026
@iscekic
iscekic merged commit bf482f2 into main Jul 28, 2026
20 checks passed
@iscekic
iscekic deleted the active-search-08a5 branch July 28, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

human-ready The PR is ready for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants