Skip to content

fix(server): read the smallest iOS simulator text size as small - #22114

Open
only21mil wants to merge 1 commit into
pingdotgg:mainfrom
only21mil:r9/g14-1
Open

only21mil wants to merge 1 commit into
pingdotgg:mainfrom
only21mil:r9/g14-1

Conversation

@only21mil

Copy link
Copy Markdown
Contributor

Problem

When an iOS simulator uses the smallest Dynamic Type category, extra-small, the Device panel reports its text size as Large. The reader matches the extra substring before reaching its explicit extra-small check.

Change

Check the small categories before the broader extra match. Add a regression test through readDeviceDetail using extra-small\n as the simulator command output.

Scope and approval

This is a very small, focused fix for an obvious bug under CONTRIBUTING.md's exception. The existing reader already lists extra-small as a small category, but its earlier branch prevents that mapping. No prior issue or discussion is linked.

Verification

  • With the regression test added to the unchanged base, vp test run src/device/DeviceActions.test.ts -t 'reads the smallest iOS text size as small' --maxWorkers=1 fails with expected small, received large.

  • vp test run src/device/DeviceActions.test.ts --maxWorkers=1 passes all 18 tests on the branch, run from apps/server.

  • node node_modules/typescript/bin/tsc --noEmit -p apps/server/tsconfig.json passes from the repository root.

  • vp fmt --check apps/server/src/device/DeviceActions.ts apps/server/src/device/DeviceActions.test.ts passes.

  • vp lint apps/server/src/device/DeviceActions.ts apps/server/src/device/DeviceActions.test.ts passes.

  • UI verification on an iPhone 17 Pro simulator running iOS 26.5. Set content_size extra-small with simctl; the real command reads back extra-Small. The Device panel on main checks Large. The candidate checks Small against the same simulator and unchanged setting. Restored the original text size and cleaned up both dev servers and the simulator.

Main incorrectly checks Large for the smallest iOS Dynamic Type category

The fix correctly checks Small for the same iOS Dynamic Type category

Not checked:

  • The repository-wide test suite and CI.
  • Android text-size mapping; this change only alters the iOS categories.

Prior PRs: No earlier PR for this bug was found. #21835 changes the device-host test helper without changing this mapping, and its only overlap with this patch is the makeReady helper, so either merge order applies cleanly. The merged #20182 and #21996 cover Android rotation and tracing, respectively. This fix reuses no changes from those PRs.

Production fix, tests, PR description and images written with GPT-6.1 Sol in T3 Code Nightly.

🤖 Generated with GPT-6.1 Sol in T3 Code Nightly

Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 20, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 6c83bce

Macroscope's review found this PR approvable — This is a narrowly scoped server bug fix that corrects the iOS simulator extra-small text-size classification and adds a regression test. It does not alter product defaults, schemas, workflows, or static-analysis settings.

Notes:

  • Code review disabled. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 61e4b7be-0f2b-4ff2-9e81-8a5cf6717696
📥 Commits

Reviewing files that changed from the base of the PR and between 143ab16 and 6c83bce.

📒 Files selected for processing: 2
  • apps/server/src/device/DeviceActions.test.ts
  • apps/server/src/device/DeviceActions.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

📝 Walkthrough

The iOS text-size mapping now checks exact categories before applying the broader extra fallback. A regression test verifies that extra-small maps to small.

Changes: iOS text-size mapping

Layer / File(s) Summary
Correct category mapping and regression coverage
apps/server/src/device/DeviceActions.ts, apps/server/src/device/DeviceActions.test.ts
The mapper checks extra-small, small, and medium before the broader extra fallback. The regression test verifies that extra-small maps to small in detail.settings.textSize.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 6c83b

The PR correctly reports iOS extra-small text size as Small and adds regression coverage.

Architecture Summary

Architecture risk: 🔵 Low · up to 6c83b

The change affects 1 system.

Changed systems: apps/server

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — apps/server (service) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/server/src/device/DeviceActions.test.ts: Added coverage confirming that iOS extra-small content-size output maps to small in detail.settings.textSize.
  • observed — Modified behavior in apps/server/src/device/DeviceActions.ts: textSizeFromIos now maps "extra-small", "small", and "medium" directly to "small" before mapping categories containing "extra" to "large"; the previous ordering tested "extra" first.
Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary change: correcting the iOS simulator mapping for the smallest text size.
Description check Passed The description includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the bug, implementation, exception rationale, focused test results, manual UI verific…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the bug, implementation, exception rationale, focused test results, manual UI verification, and unverified areas.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

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

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant