Skip to content

fix(mobile): agents session-list scroll behavior + session platform icons - #4773

Merged
iscekic merged 8 commits into
mainfrom
feat/mobile-session-list-ux
Jul 27, 2026
Merged

fix(mobile): agents session-list scroll behavior + session platform icons#4773
iscekic merged 8 commits into
mainfrom
feat/mobile-session-list-ux

Conversation

@iscekic

@iscekic iscekic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Batch B of the Mobile Agents session-list UX work — two confirmed defect fixes, one scroll-behavior hardening, and one feature, shipped as four coherent commits:

  1. Back-navigation scroll reset (defect, reproduced at the gate). Scrolling deep into the Agents list, opening a session, answering a live question (ack), and going back reset the list to the top. Root cause: the SectionList was keyed on ${sortBy}:${attentionFocusRevision}, so any attention-revision change while the frozen list was away remounted it on return and dropped the scroll offset. The remount was introduced by Notify and surface when a remote/cloud-agent session needs input #4605 to refresh ack state in frozen cells, but rows already self-subscribe via useSessionAttentionRevision(). The list now keys only on sortBy (sort changes still remount, as before) and passes the focus-snapshotted attention revision as extraData, which re-renders visible cells after unfreeze without remounting.
  2. Agents tab re-tap scrolls to top (defect, reproduced). Re-tapping the active Agents tab did nothing. Wired useScrollToTop from @react-navigation/native to the list ref — standard iOS tab-bar behavior, animated, composing with the existing haptics-only tabPress listener.
  3. Scroll-to-top on committed search query (hardening; the reported defect did not reproduce at the gate). The repro gate could not confirm "committing a new search query resets scroll to top" as a baseline defect, and the gate evidence was inconclusive for the list content component, which has no baseline mechanism that resets the offset on query commit (no ref, no effect, no remount path). The behavior is therefore shipped explicitly: a prevSearchQueryRef effect scrolls the list to top (non-animated) when the committed, debounced query changes — never on initial mount, focus refetch, attention revision, sort remount, pagination, pull-to-refresh, or section-data identity changes with an unchanged query. The selection decision lives in the pure shouldResetScrollOnCommittedQuery() helper with unit coverage; the acceptance E2E runbook carries the search regression assertion.
  4. Session origin platform icon (feature). Known-platform sessions now show a subtle monochrome icon at the right of the card eyebrow in the Agents list (12px, muted, purely decorative with accessible={false}), and the same icon on the session detail header between the back button and the title (14px, non-interactive; exposed to screen readers as an informative image labeled with the platform name, since the detail header carries no other platform text). Mapping: cloud-agent/cloud-agent-web → cloud, cli → terminal, vscode/agent-manager → code, slack, github; unmapped/unknown platforms render nothing by design. SlackIcon gained a backward-compatible monochrome color variant. Rows in NEEDS INPUT state keep the attention treatment with no icon. In list rows, VoiceOver appends from <PLATFORM> only when the eyebrow badge is a repo name (otherwise the badge already speaks the platform). Data for the detail icon comes from an additive created_on_platform field on cliSessionsV2.getWithRuntimeState (web behavior unchanged).

Verification

  • Repro gate evidence (baseline): the back-navigation scroll reset reproduced via steered ack flow (/var/folders/pz/_kmbp8vs2755j415slh2hz100000gn/T/kilo/repro2-ack-scroll.ksZpPA/); the Agents tab re-tap reproduced (/var/folders/pz/_kmbp8vs2755j415slh2hz100000gn/T/kilo/session-list-repro.ehpQbw/); the search-query scroll reset did not reproduce and is shipped as hardening (item 3).
  • pnpm format && pnpm typecheck && pnpm lint && pnpm check:unused && pnpm test (apps/mobile: 228 files, 1891 tests) and root pnpm run typecheck && pnpm run lint && pnpm run format:changed all green.
  • Unit coverage: platform-icon mapping, eyebrow-right icon-slot decision (incl. needs-input suppression and byte-identity for existing callers), accessibility-label platform segment (fixed literal inputs), committed-query scroll-reset selection logic (mount / unchanged / changed / empty ↔ non-empty).
  • Device E2E on the claimed iOS simulator runs against the local seeded stack per the plan's acceptance runbook (back-nav preservation incl. the ack flow, status-flip attention guard, search regression, tab re-tap, list + detail icon identifiers, coexistence).

@iscekic iscekic self-assigned this Jul 26, 2026
@iscekic

iscekic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

(bot) Standin review — posted because the automated reviewer is broken: the Kilo Code Review status check runs green on this head but never posts a review to GitHub, and the @kilocode-bot retrigger path has been abandoned by the requester. This review was produced by a dedicated review agent (mobile-reviewer, fresh session, read-only) over the full PR diff (git diff origin/main...604dc42802744e5b47921eed148d956b4d8fec72). It is not a Kilobot review.

Verdict: 3 findings (0 blocker, 0 major, 1 minor, 2 nit)

Findings

  1. Minor — apps/mobile/src/components/agents/session-list-content.tsx:88-103 (+ session-list-screen.tsx:312): search-scroll disclosure is inaccurate; the shipped behavior has no automated coverage.
    The PR description discloses "committing a new search query resets scroll to top" as a dropped item, stating commit a00608cb2 "carries only a regression assertion" because "the behavior already exists on the baseline." Verified against the code: the commit adds new runtime behavior — a prevSearchQueryRef effect that imperatively scrolls the list to top on committed-query change — and adds no test or assertion of any kind (commit touches only the two component files). The baseline had no scroll-reset mechanism: no ref, no effect, and AgentSessionListScreen renders AgentSessionListContent unconditionally (no remount path on query commit), so the "already exists on baseline" claim does not hold for the content component. The effect itself is correct (skips initial mount, fires only on committed-query change, doesn't interfere with the sort remount or the extraData path) and matches the original Batch-B item's intent — but the PR ships a behavior change while disclosing a no-op, which matters for reviewers and release notes. Required outcome: correct the PR description to state the behavior is newly implemented, and either add coverage or record an accepted rationale for its absence.

  2. Nit — apps/mobile/src/components/ui/session-row.tsx:123-133: the icon wrapper defeats the meta Text's shrink.
    When showPlatformIcon is true and branchContent is non-null, icon + branch are wrapped in <View className="flex-row items-center gap-1.5"> with no shrink/min-w-0. Flex-shrink only acts within a constrained flex container, so the meta Text's existing shrink class (which previously truncated long metas against the eyebrow row) can no longer shrink; a long repo-name eyebrow plus icon plus meta overflows instead of truncating. Practical risk is low — visible metas are short (timeAgo().toUpperCase() with optional $0.12 · prefix) — but the shrink was placed deliberately. Required outcome: the icon+branch cluster remains shrink-constrained so the meta truncates rather than overflows in the icon-present case.

  3. Nit — apps/mobile/src/components/agents/session-detail-content.tsx:491-500: detail-header icon described as "decorative" but is exposed to screen readers.
    The wrapper sets accessibilityRole="image" and accessibilityLabel={platformLabel(platform)}, so VoiceOver/TalkBack stops on and announces the icon. The detail ScreenHeader receives no eyebrow or other platform text, so the label is the only spoken platform indication — arguably desirable, and no acceptance criterion is violated, but it contradicts the PR description's "non-interactive, decorative" characterization. Required outcome: align the description with the implementation (labeled, informative image) or make the icon truly decorative (accessible={false}); either is acceptable, they should just agree.

Verified explicitly (no issues found)

  • Scroll-preservation fix (session-list-content.tsx:153-165, 273-294): key={sortBy} preserves scroll across back-navigation (no remount on attention-revision change); extraData={attentionFocusRevision} re-renders visible cells after unfreeze per the VirtualizedList extraData contract; rows re-read the ack store via their own useSyncExternalStore, so no staleness from the focus snapshot; sort changes still remount; all hooks precede the early returns.
  • useScrollToTop: verified against installed @react-navigation/native@7.1.33 source — SectionList ref resolves via getScrollResponder(); scroll fires only when the screen is focused and is the tab stack's first route, and only when the press is not default-prevented. The Agents tabPress listener calls no preventDefault(), so haptics and scroll-to-top compose. Ref survives the key={sortBy} remount.
  • Platform-icon mapping / slot logic: mapping matches the spec exactly; needs-input suppression enforced in selectSessionRowEyebrowRight and mirrored in the a11y gating; repo-badge gating uses the same repoNameFromGitUrl derivation as storedSessionEyebrowLabel; Home (variant='card') and remote-session-row.tsx never pass platformIcon, so their render path is byte-identical to baseline.
  • created_on_platform end to end: DB column is text().notNull().default('unknown'), so the new z.string() output field cannot reject; additive to getWithRuntimeState only; web FetchedSessionData.createdOnPlatform is optional and populated solely by the mobile adapter; existing web consumers unaffected; the list endpoint already selected the field, feeding StoredSession.created_on_platform for the list icon.
  • AGENTS.md compliance: named exports only; Tailwind className only; Lucide icons colored via existing theme tokens; SlackIcon's color prop optional with brand fills preserved; no re-parsing of trusted tRPC data; no new date parsing; decorative list icon wrapper uses accessible={false}.
  • Tests assert behavior (mapping table, slot decision incl. needs-input suppression and undefined→false byte-identity, spoken-label segment order and gating), not implementation trivia.
  • Checks run from apps/mobile/: pnpm typecheck, pnpm lint (0 problems, 891 files), pnpm test (227 files / 1887 tests), pnpm check:unused, pnpm format:check — all green. Empty retrigger commits 3e41f06f6/604dc4280 confirmed content-free.

Triage and repairs for the findings above follow in a subsequent comment.

iscekic added 2 commits July 26, 2026 14:49
The platform-icon + branch wrapper in the session-row eyebrow-right slot
had no shrink constraint, defeating the inner meta Text's shrink: a long
repo-name eyebrow plus icon plus meta overflowed instead of truncating.
Add shrink to the wrapper only; pre-existing wrappers are untouched.
…it coverage

The committed-query scroll-to-top effect shipped without automated
coverage. Extract the selection decision into the pure
shouldResetScrollOnCommittedQuery(prev, next) helper — false on initial
mount and on unchanged query, true on any committed change — and cover
the branches with unit tests. Effect behavior is unchanged.
@iscekic

iscekic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

(bot) Triage of the standin review above — all three findings verified valid against the code and repaired. Repairs are pushed as head 6b5d0439e (two commits).

Finding 1 (minor — search-scroll disclosure/coverage): valid. Repaired in two parts:

  • PR description corrected. The item is no longer disclosed as dropped: it is now item 3, "Scroll-to-top on committed search query (hardening)", describing the actual prevSearchQueryRef mechanism, the inconclusive repro-gate history, and the explicit no-fire cases (mount, refetch, attention revision, sort remount, pagination, pull-to-refresh).
  • Coverage added in 6b5d0439e: the selection decision is extracted into the pure shouldResetScrollOnCommittedQuery(prev, next) helper (false on initial mount and on unchanged query, true on any committed change including empty ↔ non-empty) with a 4-case unit test. Effect behavior is byte-for-byte equivalent (ref is assigned on every run; the scroll call is unchanged). A focused device-E2E pass on the repaired head is running and re-asserts the search-commit scroll-to-top behavior on device.

Finding 2 (nit — icon wrapper defeats meta shrink): valid. Repaired in 0d341a51f: shrink added to the icon+branch wrapper only, matching the file's existing shrink pattern on the meta Text (React Native Yoga defaults minWidth to 0, so shrink alone re-constrains the cluster). The pre-existing needs-input / live-and-meta wrappers were deliberately left untouched. A fresh independent reviewer verified: no other structural change, non-icon callers stay byte-identical, and the diff cannot alter any device-E2E-verified acceptance behavior (back-nav preservation, tab re-tap, icon rendering, a11y labels, needs-input treatment).

Finding 3 (nit — "decorative" detail icon exposed to screen readers): valid as a description mismatch; resolved by description, keeping the implementation. The accessible implementation is the desirable one: the detail header carries no other platform text, so the labeled informative image is the only spoken platform indication, and no acceptance criterion is violated. The PR description now states this precisely: detail icon = 14px, non-interactive, exposed as an informative image labeled with the platform name; list icon = purely decorative (accessible={false}).

Repair checks (from apps/mobile/): pnpm format, pnpm typecheck, pnpm lint (0 problems), pnpm test (228 files / 1891 tests, incl. the 4 new cases), pnpm check:unused — all green. Fresh repair reviewer verdict: No findings.

Next: a fresh full-diff standin review of head 6b5d0439e and the focused device-E2E rerun; results will be posted here. The two empty chore(mobile): retrigger code review commits (3e41f06f6, 604dc4280) remain in history; they are content-free.

@iscekic

iscekic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

(bot) Standin review, round 2 — head 6b5d0439e. Same arrangement as before: the automated reviewer posts nothing, so this review was produced by a fresh dedicated review agent over the full PR diff (git diff origin/main...6b5d0439e). It is not a Kilobot review.

Verdict: No findings.

Round-1 repair confirmation

  1. Scroll-reset disclosure/coverage — resolved. session-list-scroll-reset.ts exports the pure shouldResetScrollOnCommittedQuery(prev, next); the test covers all 4 cases (mount/null, unchanged, change, empty ↔ non-empty) and passes. The effect assigns prevSearchQueryRef.current on every run and keeps the original scrollTo({ y: 0, animated: false }) — behavior preserved.
  2. Icon wrapper shrink — resolved. The icon+branch cluster renders in <View className="shrink flex-row items-center gap-1.5">; only that wrapper changed (0d341a51f).
  3. Detail icon a11y — resolved as intended-keep. Detail header icon remains an informative image (accessibilityRole="image" + platform label); the list icon stays decorative (accessible={false}). Matches the corrected PR description.

Verified explicitly on this head

  • Back-navigation scroll preservation: list keys only on sortBy; focus-snapshotted attention revision flows via extraData, so visible cells re-render after unfreeze without remount; in-browsing revision bumps don't disturb scroll.
  • Tab re-tap: useScrollToTop(listRef) composes with the haptics-only tabPress listener (no preventDefault); confirmed against installed @react-navigation/native@7.1.33 source (SectionList ref resolves via getScrollResponder(), gated on focused + first route).
  • Committed-query scroll reset: searchQuery is the debounced committed value; the effect fires only on committed-query change — never on mount, refetch, attention revision, sort remount, pagination, or pull-to-refresh.
  • Icon mapping and gating: mapping matches spec; unmapped/unknown/null → nothing rendered; NEEDS INPUT suppresses the icon in the selector and the a11y gating; repo-name badge gating consistent with storedSessionEyebrowLabel; Home variant='card' path structurally identical.
  • created_on_platform contract: additive on cliSessionsV2.getWithRuntimeState, backed by text().notNull().default('unknown'); old clients ignore the field, new mobile against a stale server degrades to no icon; web consumer unaffected.
  • AGENTS.md compliance: NativeWind classes only, Lucide via useThemeColors(), named exports, no inline styles, no route-type silencing.
  • Checks (from apps/mobile/, all green): pnpm typecheck, pnpm lint (0 problems, 893 files), pnpm test (228 files / 1891 tests), pnpm check:unused, pnpm format:check.

Residual testing risks (reviewer's own disclosure): this pass was unit tests plus code/library-source inspection — no on-device coverage of the scroll behaviors, icon rendering, or VoiceOver wording on this head. The focused device-E2E rerun reported separately covers exactly that surface.

This concludes the standin review loop: two rounds, three findings repaired and confirmed, no actionable findings remaining on the latest head.

@iscekic

iscekic commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

(bot) Focused device-E2E rerun on head 6b5d0439e — verdict: ACCEPT, all items PASS (iOS simulator, local seeded stack, fresh mobile-e2e-verifier).

This rerun was required because the standin-review repairs touch the session-list surface. Results:

Item Result Evidence summary
Committed search query resets list to top PASS Scrolled deep → committed "Seed session 1" → first row Seed session 11…, scrollbar at 0%
Tab switch Home → Agents preserves scroll offset PASS Deep at Check-in (scrollbar 100%) → Home → Agents → still Check-in at 100% (exclusive retest authoritative after an earlier driver-contention anomaly)
Back from session detail preserves offset PASS I2_BEFORE = I2_AFTER = Check-in; post-back screenshot still on the deep list
Re-tapping Agents tab scrolls to top PASS Deep list → re-tap → Seed session 6… at top, scrollbar 0%
Platform icons on non-attention rows PASS platform-icon-terminal nodes in the list hierarchy; glyphs render correctly
NEEDS INPUT suppresses the platform icon PASS Needs-input row shows the amber attention label with 0 platform-icon-* nodes

Zero product failures. Transient test-environment issues only (concurrent Maestro driver contention, a foreign simulator rename mid-run — reclaimed per protocol), none affecting verdicts. The verifier made one sanctioned temporary edit (a login-flow settle regex for an iOS dialog variant) and restored it byte-for-byte (SHA-256 verified); the worktree ended clean at 6b5d0439e with no product edits. Unit suites re-run on device side: session-list-scroll-reset, session-row-eyebrow-right, session-platform-icon (23/23 pass).

One disclosed residual: an explicit long-repo-name eyebrow + icon overflow/truncation assertion was not produced on device (no suitable seeded session); that edge is covered by the shrink constraint itself, the byte-identity unit tests, and the static review. All E2E resources from this run (stack, remote CLI, simulator, device slot) have been shut down or released.

Completion status from this workflow's side: standin review loop closed (round 2: no findings), CI green on 6b5d0439e, PR mergeable and assigned. The PR now awaits human review — that is the intended end state; no further bot action unless new findings appear.

@iscekic
iscekic enabled auto-merge (squash) July 26, 2026 15:54
@iscekic
iscekic disabled auto-merge July 26, 2026 15:58
@Kilo-Org Kilo-Org deleted a comment from kilo-code-bot Bot Jul 26, 2026
@iscekic
iscekic merged commit 12ec3c6 into main Jul 27, 2026
19 checks passed
@iscekic
iscekic deleted the feat/mobile-session-list-ux branch July 27, 2026 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants