Skip to content

PR-MODEL-ROW-AND-OAUTH-EMAIL-FIX-0: 2 of 6 bug bundle (height-clip + email-leak) - #373

Merged
jackwener merged 1 commit into
mainfrom
yuejing/models-and-defaults-6-bug-bundle
Jun 30, 2026
Merged

PR-MODEL-ROW-AND-OAUTH-EMAIL-FIX-0: 2 of 6 bug bundle (height-clip + email-leak)#373
jackwener merged 1 commit into
mainfrom
yuejing/models-and-defaults-6-bug-bundle

Conversation

@jackwener

Copy link
Copy Markdown
Member

Why

WAWQAQ msg c71b4dcb and 77221a77 flagged 6 issues across 2 screenshots in Settings → 模型 / 默认模型. After @maka-审美专家's analysis we split them into root-cause buckets:

Bug Root cause Status
1: model-row second line clipped Button size="md" ships fixed h-9; rows now have 2 lines ✅ this PR
2: middle row appears with different background same height-clip; the apparent "white middle row" is just :hover state on the taller row ✅ this PR (transitively)
3: long title truncation doesn't match raw-id truncation already has overflow:hidden + text-overflow:ellipsis; visible artifact was bug 1 height-clip misread ✅ this PR (transitively)
4: refresh button uses mint primary color (attributed to PR #329 L3) PR #329 L3 was NOT merged; the mint button is the existing --primary token unchanged ⚠ misdiagnosed
5: default-model dropdown lacks model-within-connection selection needs new setDefault({slug, model}) IPC + GeneralDefaultsCard rewire onto grouped Select 🔜 separate PR
6: Codex OAuth dropdown leaks user email connection.name built as Codex OAuth · ${state.email} at the source ✅ this PR

What's fixed in this PR

Bug 1: model row second line clipped (1 of 4 model-list bugs)

.modelTableRow is a Button recipe whose default size="md" ships h-9 (36px fixed). Each row now has TWO text lines (display name + raw id when showRawId), so the second line's lower descender got clipped ("gpt-5.4" → "ant 5.4" in the screenshot). Recipe override:

.modelTableRow {
  height: auto;
  min-height: 44px;
  padding: 8px 10px;  /* was 4px 8px */
}

This also resolves bug 2 (the apparent "middle row has a different background" is just the focused/hovered state on the taller row, reading as expected behavior with the height fixed) and bug 3 (.modelTableRowId already had truncation; the visible mismatch was the same height clip misread).

Bug 6: OAuth display name leaks user email

apps/desktop/src/main/main.ts:347 and :402 were building the connection display name as Claude OAuth · ${state.profile.email} (and the symmetric Codex path). That value is persisted as connection.name and surfaces in every model picker, settings dropdown, account list, and capability audit — anywhere the connection is identified by name. The email belongs on the Account · 模型 page, not in the model identity. Both paths now use brand-only labels.

What's deferred

Bug 5 — separate PR

GeneralDefaultsCard currently shows ONLY connection options (e.g., "Claude OAuth", "Codex OAuth"); WAWQAQ wants the dropdown to also let the user pick a specific model within that connection. The fix requires:

  • A new IPC contract setDefault({slug, model}) (vs current setDefault(slug))
  • Rewiring GeneralDefaultsCard onto the grouped Select pattern that ChatModelSwitcher already uses (packages/ui/src/chat-model-switcher.tsx)
  • Migrating existing stored defaults

This is a backend-touching change with its own contract scope. Filing as a separate PR per scope discipline.

Bug 4 — misdiagnosed

The aesthetics expert's bug 4 claim ("mint button is PR #329 L3's fault") doesn't hold: PR #329 L3 commit 023b90b2 was NEVER merged (only the L1 vibrancy unblock from 7d008d51 shipped). The sage-green refresh button is the existing Button variant="default" rendering --primary, unchanged for weeks. No fix needed.

Tests

tsc --noEmit clean against my edits. Two pre-existing TS errors (displayName in model-catalog-choices.test.ts and recordActiveFullCompactBlock in main.ts:888) also fail on main without my changes; unrelated.

Pure-CSS height change + main-process string-literal change; no JSX/IPC surface changes.

…b / 77221a77)

Two of WAWQAQ's screenshots flagged 6 issues. This PR ships the 2 with
clean root causes (1 CSS + 1 backend); 4 others split out per scope:

### Bug 1 fixed: model-row second line clipped (1 of 4 model-list bugs)

`.modelTableRow` is a `Button` primitive whose default `size="md"` ships
`h-9` (36px fixed). Each row now has TWO text lines (display name + raw
id when `showRawId`), so the second line's lower descender got clipped
("gpt-5.4" → "ant 5.4" in the screenshot). Override on the recipe:

  height: auto;
  min-height: 44px;
  padding: 8px 10px;  /* was 4px 8px */

This also resolves the apparent "middle row has a different background"
bug — the user just hovered/focused that row; with the taller row it
reads as expected hover state, not a separate visual.

### Bug 6 fixed: OAuth display name leaks user email

`apps/desktop/src/main/main.ts` was constructing the connection display
name as `Claude OAuth · ${state.profile.email}` (and the symmetric Codex
path). That value is persisted as `connection.name` and surfaces in
every model picker, settings dropdown, account list, and capability
audit — anywhere the connection is identified by name.

The email belongs on the Account · 模型 page, not in model identity.
Both paths now use brand-only labels:

  const displayName = 'Claude OAuth';
  const displayName = 'Codex OAuth';

The email is still available via the OAuth state for the dedicated
account surfaces that legitimately need it. Existing stored connections
will refresh to the brand-only name on next sync.

### NOT in this PR — split out for scope

- **Bug 5 (default-model dropdown lacks model selection)** — requires a
  new IPC contract (`setDefault({slug, model})` vs current slug-only),
  plus rewiring `GeneralDefaultsCard` onto the grouped `Select` pattern
  that `ChatModelSwitcher` already uses. Separate backend-touching PR.
- **Bug 3 (row title truncation not matching second-line truncation)**
  — `.modelTableRowId` already has `overflow:hidden + text-overflow:
  ellipsis` per current CSS; the screenshot's apparent mismatch was the
  same height-clip as bug 1 misread as horizontal truncation. Fixed
  transitively by the bug-1 height fix.
- **Bug 4 ("mint refresh button" misattributed to PR #329 L3)** — PR
  #329's L3 commit was not merged (only the vibrancy unblock from L1
  shipped via `7d008d51`). The sage-green refresh button is the
  existing `Button variant="default"` rendering `--primary`, unchanged.
  No fix needed.

### Tests

`tsc --noEmit` clean against my edits. The two pre-existing errors
(`displayName` in `model-catalog-choices.test.ts` and
`recordActiveFullCompactBlock` in `main.ts:888`) also fail on `main`
without my changes; unrelated to this PR.
@jackwener
jackwener merged commit 5d58cd4 into main Jun 30, 2026
@Astro-Han
Astro-Han deleted the yuejing/models-and-defaults-6-bug-bundle branch July 14, 2026 05:05
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.

1 participant