Skip to content

fix(mobile): keep one profile picker header and scope its fixture link - #9436

Open
iscekic wants to merge 9 commits into
mainfrom
drain/profile-picker-fixture-tooling
Open

iscekic wants to merge 9 commits into
mainfrom
drain/profile-picker-fixture-tooling

Conversation

@iscekic

@iscekic iscekic commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

Evidence

Evidence: demonstrated — ran the design gate and the development fixture links on this VM's Android emulator against the local dev stack, after merging main into the branch. At this head both fixture scopes render one header (Cancel, the centred Pick a profile title, Done) and the empty state names its condition; on the merge base the same personal-scope link renders the sheet with no Cancel and repeats the sheet title as the empty-state title. Read from the gate's own dumps of each state, not from the screenshots alone.

What changes

main now carries most of what this branch originally added — the profile picker fixture, its seed topic, the form sheet header's window-inset decision, and the design-gate clipping fix — so the merge takes main's versions of those files and this pull request keeps the two pieces that are still its own.

  • apps/mobile/src/components/agents/profile-picker-sheet.tsx keeps one header across the sheet's states. The list state passed Cancel while the loading, error and empty states did not, and the shared header centres its title only between a leading and a trailing control, so those states redrew the same sheet with a leading title and no way out at the start. All four states now pass Cancel, and the empty state names the empty condition over the profiles screen's existing description instead of repeating the sheet title, which removes the sheet-only catalog string from every locale.
  • apps/mobile/src/lib/profile-picker-fixture.ts keeps an empty org value in the personal scope. A link whose org carried no value named an organization id of '', which the sheet answered with its organization error state instead of the personal scope. profile-picker-fixture.test.ts covers that link shape next to the others.

No production behavior changes: the fixture path is reached only through the development scheme and a __DEV__ check, and the sheet change is presentation and copy only.

Verification

  • Design gate at this head, --matrix standard, three links (both fixture scopes plus the model-picker sheet as a control): exit 0, 0 errors, 0 warnings across 4 cells; skippedRules is empty.
  • Merge-base run of the same three links, same emulator and stack: exit 0, 0 errors, 0 warnings. The comparison therefore introduces no findings; what this pull request changes is sheet-state identity and copy, which the gate's rules do not grade.
  • The gate's dumps at this head: the personal-scope link settles on /agent-chat/profile-picker with Cancel, Pick a profile, Done, then No profiles yet over the shared description; the organization link settles on the same route with the same header and the seeded shared profile as the resolved default. The merge-base dumps show Pick a profile, Done, then Pick a profile again with the longer empty-state sentence and no Cancel, which is the state this change removes.
  • Unit tests: profile-picker-sheet.mounted.test.tsx asserts one header across all four states (Cancel present, the same title, Cancel closes) plus the empty state's copy; profile-picker-fixture.test.ts covers the link shapes including the empty org value. The dev/local/design-gate suites (140 tests) pass. pnpm --filter kilo-app run check:i18n reports every one of the 87 catalogs matches en.json. pnpm --filter kilo-app run typecheck, the mobile oxlint config on the touched paths, and oxfmt --list-different on them are clean.

Design gate: 4 cells, 0 errors, 0 warnings

Production prerequisites

None.

Proxy answers

The owner's proxy gave these answers in the owner's place.

  1. Question: Design-gate findings: Both errors the host listed are sheet-wide, not fixture-specific: the clipped one is a design-gate coordinate-space bug (main's model-picker sheet reports it too), and the 51.8 dp layout-shift is a real title jump in the shared SheetHeader. To get the gate to exit 0 at the fixture head I must touch dev/local/design-gate (compare boxes in one coordinate space, with unit tests) and apps/mobile/src/components/sheet-header.tsx (Android bottom form sheets never reserve the status-bar inset, because the capped full detent always sits below it). How wide should this PR go?
    Answer: Fix both in this PR
    Confidence: 0.66 · Precedent: Put each separate concern in its own PR, but put a small fix that unblocks the current PR's CI into that PR: owner typed "fix in this PR" instead of a separate fix PR (a9522cd75f53). Also: fix the class, not the symptom, and fix gaps you see (00eb2a1d3e4b, 89824a222897).

The new-session profile picker reads its pick and its scope from the bridge
the composer writes, so a deep link alone only shows the expired state and no
design-gate run can measure the sheet.

This adds a development-only fixture path that writes the same bridge and
opens the same route, plus a seed topic that creates an organization whose
shared profile is its default, so the sheet can be opened with real data in
both scopes. No production behavior changes: the fixture is behind the
development scheme and a __DEV__ check.
@iscekic iscekic added drain Work item from the findings drain explorer:android Finding surfaced by the Android explorer labels Nov 1, 2026
@kilo-code-bot

kilo-code-bot Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change adds one test asserting that a fixture link with an empty org value resolves to the personal scope, matching the fixture's existing empty-or-absent handling, and I found no defect in the changed lines.

Assumptions: [INFERENCE] I could not execute the mobile test suite (read-only review); I statically confirmed that new URLSearchParams('org=').get('org') yields '', which profile-picker-fixture.ts already maps to undefined, and that no source file still references the removed agentChat.newSession.noProfiles key. The delta since the previous review is c917cbeca (7 test lines in profile-picker-fixture.test.ts); the origin/main merge in 078ab77b7 brought main's own commits (#9438, #9347), which I treated as out of scope.

Files Reviewed (1 file)
  • apps/mobile/src/lib/profile-picker-fixture.test.ts
Previous Review Summaries (4 snapshots, latest commit 32299d4)

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

Previous review (commit 32299d4)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change gives the profile picker sheet's loading, error, and empty states the same Cancel control as the list state so all four share one centred header, and replaces the sheet-only empty copy with the existing profiles.emptyTitle/profiles.emptyDescription keys after deleting agentChat.newSession.noProfiles from every catalog; I found no defect in the changed lines.

Assumptions: [INFERENCE] I could not execute the mobile test or i18n suites (read-only review); I statically confirmed profiles.emptyTitle and profiles.emptyDescription resolve in all 87 catalogs, every catalog parses as valid JSON, no code still references agentChat.newSession.noProfiles, and no locale retains it. HEAD is a merge of origin/main; I reviewed only the PR-scoped delta (07355ff7e plus the PR-relevant hunks the merge preserved) and treated main's own changes as out of scope.

Files Reviewed (3 areas)
  • apps/mobile/src/components/agents/profile-picker-sheet.tsx
  • apps/mobile/src/components/agents/profile-picker-sheet.mounted.test.tsx
  • apps/mobile/src/i18n/locales/*.json (87 catalogs)

Previous review (commit 4f3f6ef)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The two follow-up commits move the form-sheet header's top-clearance decision from a post-presentation measureInWindow to the window's own safe-area inset, and make the design-gate clipped rule compare parent and child boxes in the transform-free layout frame; both changes are internally consistent with useFormSheetDetents and leave the previously fixed fixture link intact.

Assumptions: [INFERENCE] sheet-header.tsx keys dropsTopInset on the raw useSafeAreaInsets().top without the statusBarHeight fallback that the retained comment (lines 82–89) says Android needs when a freshly presented sheet briefly reports top: 0; if that transient still occurs, the header can reserve then drop the inset — the shift this PR removes — though the reported 0 design-gate shifts suggest it did not occur on the measured device. [INFERENCE] In a landscape window with no top inset, useFormSheetDetents already offsets the sheet top edge by StatusBar.currentHeight, so reserving that inset in the header may add a dead band above the title; this is not measurable from the diff. I could not execute the mobile or design-gate suites (read-only review), so test and gate claims rest on static reading plus the PR's reported results, not independent verification.

Files Reviewed (6 files)
  • apps/mobile/src/components/sheet-header.tsx
  • apps/mobile/src/components/sheet-header.mounted.test.tsx
  • dev/local/design-gate/tree.ts
  • dev/local/design-gate/rules/screen.ts
  • dev/local/design-gate/rules/screen.test.ts
  • dev/local/design-gate/rules/shift.ts

Previous review (commit 2667773)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The follow-up commit 26677737b fixes the reported fixture bug: the path match is now checked independently of the optional query group, so the no-query personal-scope link reaches the route, and an empty org value maps to undefined instead of an empty-string organization scope. No remaining issues in the incremental change.

Assumptions: none — the fix was traced through profilePickerFixtureRoute and redirectSystemPath, and the seed topic and native-intent wiring are unchanged from the previously reviewed commit.

Files Reviewed (3 files)
  • apps/mobile/src/app/+native-intent.tsx
  • apps/mobile/src/lib/profile-picker-fixture.ts
  • dev/seed/app/agent-profile-org-default.ts

Previous review (commit 9ac9322)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The new dev-only profile-picker fixture cannot open its documented personal-scope link, because the regex's "no match" and "matched without a query" cases are both undefined; only the org form reaches the sheet.

Assumptions: none — the silent-drop fallthrough was traced through deep-link-destination.ts (APP_ROUTE_ROOTS excludes dev) at this HEAD.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/mobile/src/lib/profile-picker-fixture.ts 22 query === undefined conflates a non-fixture path with the no-query (personal-scope) fixture link, so kiloapp:///dev/profile-picker-fixture returns null and is dropped by mapWebPath. A ?org= link also yields organizationId: '' rather than undefined.
Files Reviewed (3 files)
  • apps/mobile/src/app/+native-intent.tsx - 0 issues
  • apps/mobile/src/lib/profile-picker-fixture.ts - 1 issue
  • dev/seed/app/agent-profile-org-default.ts - 0 issues

Fix these issues in Kilo Cloud


Reviewed by deepseek-flash-latest · Input: 53.7K · Output: 11.4K · Cached: 641.2K

Review guidance: REVIEW.md from base branch main

@iscekic iscekic added merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. labels Nov 1, 2026
The fixture matched its path and its query with one optional group, so a
link without a query looked like a path that is not the fixture and the
personal-scope link was dropped before it reached the route. An empty
`org` value also stayed a string, which the sheet read as an organization
scope with no id and answered with its error state.

Check the path match on its own, and read the query from the optional
group, so an absent or empty `org` is the personal scope.
@iscekic iscekic added merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. and removed merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head labels Nov 1, 2026
@iscekic

iscekic commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Ran the new profile picker fixture on the Android emulator against the local dev stack, signed in as the seeded organization owner.

  • The fixture link with no query opens the sheet in the personal scope. The same link left the app on Home before the scope-parsing commit, so the personal scope had no way in.
  • The fixture link with an empty org value renders the identical personal-scope sheet instead of an organization error state.
  • The fixture link carrying the seeded organization id opens the organization scope: its first row is Default profile, with the shared profile named beneath it.

The seed topic created the user, the organization with that user as its only member, and that organization-owned profile with is_default set.

Profile picker sheet opened in the personal scope by the fixture link with no query

Profile picker sheet in the organization scope with the shared profile named as the default

@iscekic iscekic added the merge:blocked:design-review Design review has not cleared this change label Nov 1, 2026
A transformed view moves its descendants and not its own frame, so a
natively presented formSheet reports its screen at the window origin
while its content sits at the detent. `clipped` compared the child's
transformed rect against the parent's untransformed one and reported the
whole sheet content as cut. Both sides now use the layout frame, the same
space the layout-shift rule already measures in.
…ow inset

On Android a form sheet header reserved the status-bar inset on its
first frame and dropped it once the sheet settled at its detent, moving
the title by the inset. The detent cap already keeps a sheet below the
status bar whenever the window reports a top inset, so the window inset
alone answers whether the header must clear the status bar itself — the
landscape window without one. The measurement that flipped the value
after the presentation is gone.
@iscekic iscekic removed the merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. label Nov 1, 2026
@iscekic

iscekic commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Design gate and fixture captures at 4f3f6ef.

The profile picker sheet, opened through the development fixture on this VM's Android emulator against the local dev stack (signed in as the seeded organization owner):

  • the link with no query opens the personal scope with the sheet's empty state;
  • the link carrying the seeded organization id opens that organization's scope, whose first row is Default profile with the shared profile named beneath it. The header title sits at the top of the sheet in both states.

The contact sheet is the standard-matrix design gate run over both fixture states and the existing model-picker sheet as a control: exit 0, 0 errors, 0 warnings across 4 cells.

Design gate contact sheet: both profile picker fixture states and the model picker sheet

Profile picker sheet in the personal scope with its empty state

Profile picker sheet in the organization scope with the default profile row

@iscekic iscekic added merge:blocked:insufficient-e2e Verification does not cover the change merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. and removed merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:blocked:insufficient-e2e Verification does not cover the change labels Nov 1, 2026
The profile picker sheet drew two different headers: the list state had
Cancel leading and a centred title, while the loading, error and empty
states had no Cancel, so SheetHeader left their title leading against the
sheet edge and the sheet changed shape as its data arrived.

Every state now renders the same header, and the empty state names the
condition instead of repeating the sheet title: "No profiles yet" over the
shared profiles description, which reuses copy the profiles screen already
translates. The sheet-only duplicate is gone from every catalog.
…fixture-tooling

# Conflicts:
#	apps/mobile/src/app/+native-intent.tsx
#	apps/mobile/src/i18n/locales/et.json
@iscekic iscekic removed the merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. label Nov 1, 2026
@iscekic

iscekic commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Design gate re-run at head 32299d4 on the Android emulator (--matrix standard, three links: the personal-scope fixture, the organization-scope fixture, and the existing model-picker sheet as a control).

The profile picker sheet now keeps one header in every state: Cancel at the start, the centred Pick a profile title, and Done at the end, measured at x 14.1 / centred / x 349.7 in a 411.4 dp window. The empty state names its condition (No profiles yet) instead of repeating the sheet title.

Design gate: 4 cells (base, dark, compact, rtl), 0 errors, 0 warnings. The merge-base run cannot open either fixture link and still reports the sheet-wide clipped and layout-shift findings on the model-picker sheet, which this pull request also fixes.

Attaching the run's contact sheet, with the merge-base sheet after it.

Design gate contact sheet at the pull request head: profile picker empty state and populated state share one header

Merge-base run of the same links: fixture links do not open and the model-picker sheet shows the findings this change fixes

@iscekic iscekic added merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. merge:blocked:judge-unavailable Risk assessment unavailable and removed merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:blocked:design-review Design review has not cleared this change labels Nov 1, 2026
Keep main's sheet-level top-inset declaration alongside this branch's Cancel
in every sheet state, and keep the fixture's link parsing that treats an
absent or empty `org` value as the personal scope.
@iscekic iscekic removed merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. merge:blocked:judge-unavailable Risk assessment unavailable labels Nov 1, 2026
…link

The fixture now keeps an empty `org` value in the personal scope, so the link
the seed prints with no organization still opens the sheet instead of its
organization error state. Assert that case next to the other link shapes.
@iscekic iscekic added merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head merge:blocked:insufficient-e2e Verification does not cover the change merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. and removed merge:blocked:kilobot-not-ready Review bot has not recommended merge at this head labels Nov 1, 2026
@iscekic

iscekic commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Design gate and sheet captures after merging main into the branch.

Exercised on this VM's Android emulator, dev client connected to the local Metro bundle, against the local dev stack, signed in as the seeded organization owner with the development fixture links, at this head:

  • The contact sheet is the whole design-gate run (both fixture scopes plus main's model-picker sheet as a control, --matrix standard): exit 0, 0 errors, 0 warnings, no skipped rules.
  • The personal-scope link opens the sheet with Cancel at the start, the centred Pick a profile title and Done, over the empty state No profiles yet.
  • The organization link opens the same header over the seeded shared profile, shown as the default.
  • The last capture is the same personal-scope link on the merge base, on the same emulator and stack: no Cancel, and the empty state repeats the sheet title.

The merge-base run of the same three links reports the clipping and header-shift errors this pull request fixes, including on the model-picker sheet whose code this branch does not change.

Design gate contact sheet for both profile picker fixture scopes

Profile picker sheet in the personal scope with the shared header and the empty state

Profile picker sheet in the organization scope with the shared header and the seeded shared profile

Same personal scope link on the merge base

Main now carries the design-gate clipping fix and the form sheet header's
window-inset decision from the sibling work, so take main's versions of those
files and keep this branch's profile picker header and fixture link parsing.
@iscekic iscekic removed merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. merge:blocked:insufficient-e2e Verification does not cover the change labels Nov 1, 2026
@iscekic iscekic changed the title test(mobile): add a development fixture for the profile picker sheet fix(mobile): keep one profile picker header and scope its fixture link Nov 1, 2026
@iscekic

iscekic commented Nov 1, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Design gate and sheet captures after the second merge of main.

Exercised on this VM's Android emulator, dev client connected to the local Metro bundle, against the local dev stack, signed in as the seeded organization owner, opening the development fixture links at this head:

  • Contact sheet: the whole design-gate run (both fixture scopes plus the model-picker sheet as a control, --matrix standard) — exit 0, 0 errors, 0 warnings, no skipped rules. The merge-base run of the same links on the same emulator and stack also reports no findings, so this change introduces none.
  • Personal scope at this head: the sheet shows Cancel at the start, the centred Pick a profile title, Done, and the empty state No profiles yet over the shared description.
  • Organization scope at this head: the same header over the seeded shared profile, shown as the resolved default.
  • Personal scope on the merge base: no Cancel, and the empty state repeats the sheet title with the longer sentence.

The sheet state claims come from the gate's own layout dumps of each capture, taken with the screenshots.

Design gate contact sheet for both profile picker fixture scopes

Profile picker sheet in the personal scope with one header and the empty state

Profile picker sheet in the organization scope with the same header

Same personal scope link on the merge base

@iscekic iscekic added merge:bot Bot-owned review; merge when ready once readiness passes and a review approves. merge:blocked:design-review Design review has not cleared this change and removed merge:blocked:design-review Design review has not cleared this change labels Nov 1, 2026
@iscekic iscekic added merge:blocked:design-review Design review has not cleared this change and removed merge:blocked:design-review Design review has not cleared this change labels Nov 1, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

drain Work item from the findings drain explorer:android Finding surfaced by the Android explorer merge:bot Bot-owned review; merge when ready once readiness passes and a review approves.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants