Repository navigation
fix(mobile): name the profile the default resolves to in the profile picker - #9438
Conversation
The Environment row on the new-session form names the profile the effective default resolves to, but the picker that row opens checked a generic "Default profile" and listed that same profile as an unrelated row, so the checked option did not say what the session would actually run on. The picker now shows the resolved default's name as the checked row's second line and reads it in the row's accessibility label, in every locale. The checked logic and the named candidate rows are unchanged. The hook exposes the resolved default profile next to its id, and the route passes its name to the sheet. A development-only fixture link opens the sheet with the real bridge so the design gate can measure it, and a seed topic creates an organization whose shared profile is its default.
|
bot: The two states, read from the accessibility tree in the same command as each capture:
The design gate contact sheet for this head is attached. |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe incremental commit only adds Assumptions: Files Reviewed (2 files)
Previous Review Summaries (5 snapshots, latest commit 984bac9)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 984bac9)Status: No Issues Found | Recommendation: Merge Executive SummaryThe incremental commit only removes the Mtavruli initial from Assumptions: Files Reviewed (1 file)
Previous review (commit b5e43cf)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental commit only capitalizes the Assumptions: Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (26 files)
Fix these issues in Kilo Cloud Previous review (commit c240085)Status: No Issues Found | Recommendation: Merge Executive SummaryThe incremental change narrows to the trailing-slash fix: the fixture matcher now accepts the optional trailing slash before a query, fragment, or end of string, and a new test covers both Assumptions: Files Reviewed (2 files)
Previous review (commit 207acc3)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe previously reported personal-scope link bug is fixed and covered by a test; the only remaining defect is that the matcher no longer accepts the trailing-slash form it accepted before this change. Assumptions: Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 49f44b5)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe profile-name change itself is sound; the only defect is in the new dev fixture, whose documented no-query personal-scope link silently never matches. Assumptions: Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Files Reviewed (91 files)
Reviewed by deepseek-flash-latest · Input: 58.1K · Output: 7.9K · Cached: 546K Review guidance: REVIEW.md from base branch |
The fixture path only matched when the link carried a query, so the bare link the docstring describes fell through to the app router and never opened the sheet. Match the path and read the organization from whatever query it has. A unit test covers the bare link, the organization link and a link that is not this fixture.
A link that ends with a slash reached the app router instead of the fixture, so the sheet did not open. Match an optional trailing slash before the query, end of path or fragment, and cover it in the fixture's test.
The secondary line and the row's accessibility label are sentences, and the translations rule keeps the initial case of the English copy. 26 catalogs began the new phrase with a lowercase word; they now start with the same capital the English string uses, with no wording change.
Georgian Mkhedruli has no case, so the sentence-initial capital the English copy carries does not apply to it. Restore the word as it was, without the Mtavruli initial letter.
|
bot: Design gate evidence for head c240085, uploaded to the design-gate bucket under the session and this head's sha:
Method note, because the base is not the bare merge base: the picker routes are formSheets that only the new-session composer instantiates, so |
|
bot: Re-ran the gate at head c240085: 4 cells, 8 errors, 0 warnings, the same two classes. Against the base run, seven findings match exactly; the eighth differs only by one level of element path — head reports the The uploaded run for this head (same bucket path, |
|
bot: Design gate artifacts for head c240085 now hold three runs side by side, under the session and this head's sha:
Comparing the head run with Mechanism, from the head run's base-cell timeline: the sheet title sits at y=77.71 about a second after the link opens and moves to y=483.05 about 450 ms later, once the native formSheet settles from full height to fit its content; the content wrapper is 862.1 dp tall at y=457.14 inside an 862.1 dp screen. The two error classes therefore come from the native sheet sizing its own frame, not from the row this change adds, and the untouched model-picker sheet on |
|
bot: Head 984bac9: Georgian keeps Mkhedruli (no sentence capital), the other 25 catalogs keep it. Catalog parity, plural families, the i18n lint and the format check pass. At this head the design gate run and its The sheet still shows the checked row with its second line naming the resolved profile, captured from the new-session form at this head. |
|
bot: Re-verified on this VM at head 984bac9 through the real composer, not only the fixture link, with the fake organization account whose shared profile is its default.
Design gate re-run at this head: 4 cells, 8 errors, 0 warnings. The bare merge base cannot open this sheet route by link (8128-module bundle, four "link did not open" warnings, 0 errors); the merge base plus the fixture reports the same 8 findings as the head run, seven identical in cell, target, rule, element and value and the eighth the same 51.8 dp header shift reported one node deeper. All three runs are uploaded under this session and this head's sha. |
The sheet reserved the status-bar inset in its header for the first frames and dropped it once the header measured its own position after the sheet settled. That moved the title and every row by the inset height after the first stable frame. The profile picker presents at the half detent, and its full detent stops below the status bar, so its header never overlaps the status bar. Declaring that up front keeps the first frame identical to the resting one.
|
bot: Design gate re-run at head 5a9a9c7, and the contact sheet of that run is attached.
|
|
bot: Design gate re-run on the committed head 5a9a9c7 (run finished 11:32:05Z, bundle served by this checkout's Metro), with the contact sheet of that run attached. The earlier run in the same upload measured the same working tree before it was committed; this one follows the commit.
Artifacts under the session and this head's sha: the head run at the top, |
Resolves the import list in +native-intent.tsx by keeping both development fixture routes: this branch's profile picker fixture and main's Security Agent single-route fixture.
|
bot: Merged
Artifacts uploaded for this head: the head run at the top, |











Evidence
Evidence: demonstrated — on this VM's Android emulator against the local dev stack I opened the new-session form for an organization whose shared profile is its default, opened the Pick a profile sheet, and read the accessibility tree: before the change the checked row is a generic "Default profile" with no name while the candidate row below it is the shared profile; after the change the same checked row reads "Default profile" with a second line naming that profile and its accessibility label is "Default profile: ", which is the profile the Environment row behind it names. For the sheet's header I measured the same screen through the design gate: on the merge base plus the dev fixture the header records the status-bar inset in its first frame and drops it one sample later (51.8 dp in base, dark and rtl; 36.2 dp in compact — the cells' safe-area tops), and at this head the first frame after the link opens is already the resting frame, so no cell reports movement.
What changes
apps/mobile/src/components/agents/use-effective-agent-profile.tsresolves the effective default's profile object next to its id (effectiveDefaultProfile), reusing the profile list the hook already builds.apps/mobile/src/app/(app)/agent-chat/profile-picker.tsxpasses that profile's name to the sheet.apps/mobile/src/components/agents/profile-picker-sheet.tsxrenders the resolved name as the checked row's second line, in the same secondary style the candidate rows use for their counts, and puts the name in the row's accessibility label. The checked logic, the named candidate rows, and the empty, loading, error and stale-override states are unchanged; the generic "Default profile" label stays when the resolved name is unknown.PickerSheetthat the surface never reaches the window top (SHEET_TOP_INSET,topInset="never"). The sheet's full detent stops below the status bar, so the header's status-bar reservation was dead space that arrived a frame late and moved the title and every row once the sheet settled; declaring it up front removes the movement without changing the resting frame.agentChat.newSessioninen.json, translated into the other 86 catalogs (defaultProfileResolvesTo,defaultProfileResolvesToAccessibility). Key sets, placeholders, plurals and formatting passcheck-catalogs,check:plurals,check:i18nandoxfmt.app:agent-profile-org-defaultthat creates an organization whose shared profile is its default. The fixture matches its path with or without a query or trailing slash and is covered byprofile-picker-fixture.test.ts. Both files are also carried by the prerequisite pull request fix(mobile): keep one profile picker header and scope its fixture link #9436; once that one merges,mainis merged in, this branch keeps its two link fixes in those files, and the emulator proof and the design gate are re-run at the resulting head so this pull request is left with the fix alone and a plain merge-base comparison.Verification
pnpm --filter kilo-app run typecheck, the mobile oxlint configuration,format:check,check:plurals,check:i18nandcheck:unused: clean.pnpm --filter kilo-app exec vitest run src/components/agents src/app: 322 files, 5515 tests pass. The sheet tests cover the named default row, the fallback to the generic label when the name is unknown, and the "No profile" row staying unnamed when no default applies.pnpm dev:seed app:agent-profile-org-default org-profile@example.com, thenagentProfiles.listCombinedfor that organization returnedeffectiveDefaultIdnaming the organization's shared profile and no personal default.Pick a profile: <the shared profile>. Opened the sheet, then captured the accessibility tree before and after the fix:Default profile, checked, no second line; row two the shared profile, unchecked;Default profile: <the shared profile>, checked, with the visible second lineResolves to <the shared profile>; row two unchanged.git status --porcelainempty afterwards), so both states come from the same account, organization and screen.Pick a profile: <the shared profile>; the sheet's first row is aRadioButtonwithchecked=trueand labelDefault profile: <the shared profile>, its visible second lineResolves to <the shared profile>, and the named candidate row below it keepschecked=false.sheets/dev-profile-picker-fixture.png) and the run'scontact-sheet.pngare attached to this pull request; both stay outside the checkout.Design gate: 4 cells, 4 errors, 0 warnings at head
f4f173d46— exit code 1, and the four errors are the sameclippedfinding in every cell, identical to the base run. The run was repeated after mergingmaininto the branch, so it belongs to the current head.The gate does not exit 0 for this route and cannot: the remaining error is
clippedon react-native-screens' ownRNSScreenContentWrapper, and the numbers show it is the native sheet's presentation offset rather than any frame this code lays out. In the base cell the wrapper'slayoutRectis[0, 0, 411.43, 862.1]— byte-identical to its parentRNSScreen— while its paintedrectis[0, 457.14, 411.43, 862.1]; 457.14 is exactly half of the 914.29 dp window, i.e. the half detent, and the reported 457.1 dp is that offset. Nothing inPickerSheet,SheetHeaderor this screen can change it, and it is unchanged frommain: the base run reports the same element with the same 457.1 dp (320 dp in compact, that cell's offset).That this is the formSheet shell rather than this screen is measured at this head on an untouched route: the gate run over
kiloapp:///dev/model-picker-fixture/multiple-providers, whose files this branch does not touch, reports the sameclippedon the sameRNSScreenContentWrapperin all four cells (52.2 dp, 36.2 dp in compact) — and it also still reports theSheetHeaderlayout shift this branch fixed here (51.8 / 51.8 / 36.2 / 51.8 dp), which is what confirms the shift's cause at a second, unmodified site. Fixing either for the shell is a change to the shared sheet surface, not to this work item; these stay for normal triage.Why the header moved, and the fix: the shift the base run measures equals the cell's top safe area exactly — 51.8 dp in base, dark and rtl, 36.2 dp in compact, against safe-area tops of 51.81 dp and 36.19 dp.
SheetHeaderstarts by assuming it reaches the window top and reserves the status-bar inset, then drops the inset once it measures its own position after the sheet settles (reachesWindowTop). This sheet presents at the half detent and its full detent stops below the status bar, so it never overlaps the status bar and the inset was dead space in the first frame only.ProfilePickerSheetand the route's expired state now passtopInset="never"(SHEET_TOP_INSETinapps/mobile/src/components/agents/profile-picker-sheet.tsx), which states that up front so the first frame is identical to the resting one. Measured at the full detent on the emulator, the header's top sits at 66.3 dp while the status bar is 51.8 dp, so nothing is lost by not reserving it.The upload for this head holds three runs: the head run at the top,
base/andmerge-base-without-fixture/.base/is the merge base5ee9af9cbplus the fixture from the prerequisite #9436, with the sheet code untouchedmain(applied locally on a detached checkout, never pushed on this branch). It states the deviation plainly because it is the only run that can compare:maincannot open this formSheet route through a link at all, whichmerge-base-without-fixture/records — the bare merge base reports all four links as not opened, 0 errors and 4 warnings, so it measures nothing on the changed screen. Againstbase/, the head run's fourclippedfindings are identical in cell, target, rule, element and measured value, and the fourlayout-shiftfindings the base run reports onSheetHeaderare gone at the head in the same cells. The base run attributes that shift one node deeper in one cell (… View[0] > Paragraph) than in the others (… View[0]); either way it is the header moving by a cell's safe-area top, and the head run no longer moves it anywhere.All three runs in this upload were produced with this head's bundle: the head run (4 errors, no
layout-shift), the bare merge base (all four links unopened, 0 errors, 4 warnings) and the merge base plus the fixture (8 errors). The head run and both comparison runs were repeated aftermainwas merged into the branch atf4f173d46, and the composer flow was re-checked on the emulator at that head.The mechanism is in the two timelines for the base cell. In the base run the header's first record is 125.33 dp tall with its title at y=77.71 dp, and the next record is 73.52 dp tall with the title at y=25.9 dp: 51.8 dp, the base cell's safe-area top, applied and then removed. At head the first record after the link opens is already 73.52 dp tall with the title at 25.9 dp, so there is no movement and the resting frame is unchanged.
Production prerequisites
None.
Proxy answers
The owner's proxy gave these answers in the owner's place.
Answer: Secondary line "Resolves to "
Confidence: 0.75 · Precedent: Keep similar elements consistent across surfaces: owner asked that "the thought row match the tool call row" (session a247f3c19b1e).
Answer: Commit both
Confidence: 0.75 · Precedent: Fixtures must be dev-only, faithful to real usage, and "committed for reuse" (session b1be07191a1d). Design gate chosen in session 6341f41c09ab.
Answer: Organization default
Confidence: 0.90 · Precedent: Require valid end-to-end proof: "logs and/or screenshots are mandatory proof ... make sure they make sense" (session 55c9c80332a7). The brief names a shared organization default.
Answer: Side PR with the fixture first
Confidence: 0.50 · Precedent: Never accept blocked as an end state, and stop design defects with a design gate so the owner does not need design-fix sessions: session 55c9c80332a7 "blocked is not an acceptable end state"; session 6341f41c09ab "I don't want to have those" (design-fix sessions).
Answer: Publish both runs side by side
Confidence: 0.45 · Precedent: Never accept 'blocked' as an end state, and fix the reviewer's finding (session 55c9c80332a7: "blocked is not an acceptable end state"); require valid end-to-end proof that makes sense (session 55c9c80332a7: "logs and/or screenshots are mandatory proof ... make sure they make sense"). Publishing both keeps base/ as the literal merge base and still gives the critic a real before/after without stalling on fix(mobile): keep one profile picker header and scope its fixture link #9436.
Answer: Comparison run back in base/
Confidence: 0.60 · Precedent: Autonomy: 'Never accept "blocked" or "failed" as an end state' (session 55c9c80332a7: "blocked is not an acceptable end state"), with honest disclosure per proof rules ('never change or fake results', session df631a49d5e9)
Answer: Sync main and re-verify after the merge
Confidence: n/a · Precedent: Fixtures stay committed for reuse; proof must match the head it is attached to.