Skip to content

chore(lint): keep mobile theme escape-hatch allowlist honest - #13146

Merged
juliusmarminge merged 2 commits into
mainfrom
agent/mobile-audit-theme-lint
Sep 23, 2026
Merged

juliusmarminge merged 2 commits into
mainfrom
agent/mobile-audit-theme-lint

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

Problem

The mobile lint rule t3code/no-mobile-uniwind-theme-escape-hatches keeps theme styling on semantic Uniwind classNames and only exempts reviewed native/third-party interop boundaries via an allowUniwindTheme override list in vite.config.ts. The audit of that boundary found it had drifted:

  • features/terminal/ThreadTerminalRouteScreen.tsx was still allowlisted even though the shared terminal header refactor removed its last theme-bridge usage.
  • ThreadFilesTreeScreen still read the JS palette (useUniwindTheme()["--color-header"]) to background an ordinary Android header View, which Uniwind handles with a semantic class.
  • Review follow-up: AppearancePreviews.tsx used the JS palette only for two ordinary Text nodes (line numbers / keywords in the settings code preview) — a real ordinary-screen escape hatch, not an interop boundary.

Fix

  • Drop the stale ThreadTerminalRouteScreen.tsx entry.
  • Style the files-screen Android header with bg-header (the same semantic token home screens already use) and delete the theme read.
  • Style the appearance code preview with text-icon-subtle and a conditional text-md-link on the nested keyword spans (same tokens; the nested-Text conditional class pattern already ships in thread-search-match.tsx), keep only dynamic font metrics in style, and drop AppearancePreviews.tsx from the allowlist so future JS palette reads there are linted.

The allowlist now matches the actual useUniwindTheme importers 1:1 (24 = 24).

Audit claim checked: "the rule fails on a clean tree because src/lib/useUniwindTheme.ts is missing from the allowlist"

Not reproducible; no entry is needed for that file.

  • apps/mobile/src/lib/useUniwindTheme.ts exists; it is the hook definition, not an importer. The rule reports imports of …/useUniwindTheme, useCSSVariable, useThemeColor, and dark:/light: literals — the definition file triggers none of these (it only imports AppearancePreferencesProvider).
  • Repo-wide lint on a clean tree at the base commit passes: exit 0, zero diagnostics from this rule. A probe file importing useUniwindTheme outside the allowlist does error, confirming the rule and its overrides are actually wired.
  • Counting 26 grep hits for useUniwindTheme includes the definition file; the remaining files are exactly the allowlisted importers. Adding an allowlist entry for the definition would be dead config, so none was added.

Audit claim checked: "the allowlist includes ordinary screens" (ArchivedThreadsScreen, GitOverviewSheet, NewTaskDraftScreen, ThreadSettingsSheet, thread-list items ×2, HomeHeader, SettingsEnvironmentsRouteScreen, ConnectionsNewRouteScreen)

Each of these still has a genuine JS-color consumer that cannot take a className; verified per file against current source:

File Consumer of the theme color
ArchivedThreadsScreen, thread-list-items, thread-list-v2-items ThreadSwipeable backgroundColor → RNGH/Reanimated swipeable container styles (thread-swipe-actions.tsx); v2 routes through getThreadListV2RowAppearance and also feeds the swipeable
GitOverviewSheet, ReviewSheet, thread-file-navigator-pane, ThreadFilesRouteScreen native react-native-screens Screen style / ScreenStackHeaderConfig colors, SearchBar, SVG gradient stops
NewTaskDraftScreen, ThreadComposer T3ComposerEditor native textStyle / fallbackColor
ThreadSettingsSheet, HomeHeader, ThreadFilesRouteScreen (headerTintColor) react-navigation JS screenOptions (contentStyle, headerStyle, headerTintColor)
SettingsEnvironmentsRouteScreen, ConnectionsNewRouteScreen header action tintColor → NativeHeaderToolbar extracts child.props.tintColor and hands it to the native header item

Partial class migrations for the row-background reads (e.g. bg-thread-selected on RowPressable) were considered and skipped: those files keep their allowlist entries because of the swipeable/navigation consumers, so the change would alter high-traffic selected-row visuals (interaction class vs inline style precedence in RowPressable) without removing any escape hatch. Not safely scoped for a lint-enforcement PR.

Verification (non-visual; screenshots not applicable)

  • vp test run in oxlint-plugin-t3code: 50/50 pass (rule behavior unchanged).
  • Full repo lint (vpr lint, includes --report-unused-disable-directives): exit 0 before and after; zero no-mobile-uniwind-theme-escape-hatches diagnostics; baseline probed with a temp importer to confirm rule + allowlist are applied.
  • apps/mobile tsc --noEmit: clean.
  • No visual change: bg-header, text-icon-subtle, and text-md-link resolve to the same --color-header, --color-icon-subtle, and --color-md-link tokens the removed JS reads returned.

Model: callstack/Apex via pi

Summary by CodeRabbit

  • Style
    • Android thread file screens now use the app’s standard header background. Other platforms are unchanged.
    • Appearance previews now use the app’s standard text colors for code line numbers and highlighted keywords, keeping these elements consistent with the selected appearance.

The reviewed-boundary allowlist for the no-mobile-uniwind-theme-escape-hatches
rule still carried ThreadTerminalRouteScreen after the shared terminal header
refactor removed its theme-bridge usage, and the files screen still read the
JS palette for an ordinary Android header background. Drop the stale entry,
style that background with the bg-header semantic class instead, and re-verify
the rule stays green: every remaining allowlisted file has a real native or
third-party interop consumer (react-native-screens config, Reanimated
swipeables, SVG gradients, native markdown, navigation options), and the only
other grep hit is the useUniwindTheme definition file itself, which imports
nothing, so no new entries were needed.

Model: callstack/Apex via pi
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 22, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 22, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at f27a865

Macroscope's review found this PR approvable — This is a small lint-hygiene cleanup that replaces two ordinary theme reads with equivalent semantic classes and removes stale lint allowlist entries. It introduces no new capability, schema, product-default, deployment, or sensitive-data change.

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

@github-actions

github-actions Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.5 KiB 13.5 KiB −7 B (−0.1%) 15.1 KiB ✅
Codex Thread snapshot wire 7.0 KiB 7.0 KiB −1 B (−0.0%) 7.3 KiB ✅
Codex Live turn WebSocket wire 6.5 KiB 6.5 KiB −6 B (−0.1%) 7.8 KiB ✅
Codex Live turn WebSocket decoded 56.3 KiB 56.3 KiB 0 B (0.0%) 66.4 KiB ✅
Codex Live turn messages 10 10 0 (0.0%) 21 ✅
Claude Total thread wire 13.5 KiB 13.5 KiB +46 B (+0.3%) 15.1 KiB ✅
Claude Thread snapshot wire 7.1 KiB 7.1 KiB +1 B (+0.0%) 7.3 KiB ✅
Claude Live turn WebSocket wire 6.4 KiB 6.5 KiB +45 B (+0.7%) 7.8 KiB ✅
Claude Live turn WebSocket decoded 57.0 KiB 57.1 KiB +44 B (+0.1%) 66.4 KiB ✅
Claude Live turn messages 9 10 +1 (+11.1%) 21 ✅

Baseline: d7819c1 · PR result: f27a865 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 113.9 KiB
  • Claude decoded thread snapshot: 114.6 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

The CodeAppearancePreview read the JS palette for the line-number and
keyword colors of two ordinary Text nodes. Render them with the
text-icon-subtle and text-md-link semantic classes (same tokens, and the
nested-Text conditional class pattern already used by thread search
matches), keep only the dynamic font metrics in style, and drop
AppearancePreviews.tsx from the allowUniwindTheme override so future JS
palette reads in that file are linted. The allowlist now matches the
actual useUniwindTheme importers 1:1.

Model: callstack/Apex via pi
@macroscopeapp
macroscopeapp Bot dismissed their stale review September 22, 2026 23:16

Dismissing prior approval to re-evaluate f27a865

@juliusmarminge

Copy link
Copy Markdown
Member Author

Fixed the reviewer-audit P2: AppearancePreviews.tsx rendered its code-preview line numbers/keywords with JS palette reads on ordinary Text nodes. Migrated to text-icon-subtle / conditional text-md-link (nested-Text conditional classes already ship in thread-search-match.tsx), removed its allowlist entry, and verified allowlist == importers 1:1. Head: f27a865.

@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: aafc80b8-fdb1-46f2-a8ce-5a5d09da268f

📥 Commits

Reviewing files that changed from the base of the PR and between d7819c1 and f27a865.

📒 Files selected for processing (3)
  • apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx
  • apps/mobile/src/features/settings/appearance/components/AppearancePreviews.tsx
  • vite.config.ts
💤 Files with no reviewable changes (1)
  • vite.config.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The Android files wrapper and appearance previews now use theme classes instead of colors read from the theme hook. The interop exception list no longer includes ThreadTerminalRouteScreen.tsx or AppearancePreviews.tsx.

Changes

Mobile theme class alignment

Layer / File(s) Summary
Apply and enforce theme classes
apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx, apps/mobile/src/features/settings/appearance/components/AppearancePreviews.tsx, vite.config.ts
The Android files wrapper uses bg-header. Appearance preview line numbers and keyword tokens use text-icon-subtle and text-md-link. The interop exception list removes ThreadTerminalRouteScreen.tsx and AppearancePreviews.tsx. Non-Android files route rendering is unchanged.

Priority: ⬇️ Low

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

Change: Other

Suggested reviewers: chrisdeeming

Merge Risk: ⚪ Minimal · up to f27a8

The files header and appearance preview retain their theme-based colors through semantic classes. No merge-blocking risk was found; the change is ready for normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly summarizes the main change: updating the mobile theme escape-hatch allowlist for lint accuracy.
Description check ✅ Passed The description clearly explains the problem, changes, rationale, UI impact, and verification results. It uses equivalent headings instead of the template headings and omits the explicit checklist, bu…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

@juliusmarminge
juliusmarminge merged commit 2eaff08 into main Sep 23, 2026
22 of 23 checks passed
@juliusmarminge
juliusmarminge deleted the agent/mobile-audit-theme-lint branch September 23, 2026 01:48
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Sep 23, 2026
## What's Changed
* chore(mobile): drop dead nitro-markdown tgz override and @expo/metro-runtime by @juliusmarminge in pingdotgg/t3code#13148
* feat(web): show settings scope as a sentence at the top of the page by @juliusmarminge in pingdotgg/t3code#13139
* refactor(web): move settings scope pickers into breadcrumbs by @Yash-Singh1 in pingdotgg/t3code#13165
* feat(auth): share provider sign-in flows and credential bindings by @juliusmarminge in pingdotgg/t3code#12983
* refactor(mobile): git sheets use uniwind platform variants instead of className ternaries by @juliusmarminge in pingdotgg/t3code#13161
* chore(mobile): name the two project favicon caches by their job by @juliusmarminge in pingdotgg/t3code#13160
* revert(mobile): git sheets back to Platform.OS ternaries (un-guarded uniwind variants broke both platforms) by @juliusmarminge in pingdotgg/t3code#13169
* docs(mobile): document the two mobile routes that intentionally skip deep links by @juliusmarminge in pingdotgg/t3code#13164
* refactor(mobile): break module cycles with focused extractions by @juliusmarminge in pingdotgg/t3code#13151
* fix(server): generate PR diffs from branch changes by @Yash-Singh1 in pingdotgg/t3code#13170
* fix(web): preserve nested scroll behavior in chat timeline by @Yash-Singh1 in pingdotgg/t3code#13167
* test(web): cover usage model ordering without static markup by @flamboh in pingdotgg/t3code#13104
* fix(desktop): find linuxbrew node for the WSL backend by @CodyRay in pingdotgg/t3code#7827
* chore(models): use GPT-6 Luna for text generation by @extoci in pingdotgg/t3code#13115
* fix(mobile): keep ordinary offline outbox failures out of console.warn by @juliusmarminge in pingdotgg/t3code#13144
* feat(providers): check remote compatibility ranges by @juliusmarminge in pingdotgg/t3code#13130
* chore(lint): keep mobile theme escape-hatch allowlist honest by @juliusmarminge in pingdotgg/t3code#13146
* fix(web): the pull request badge reads at the meta size again by @juliusmarminge in pingdotgg/t3code#13175
* fix(mobile): uniwind platform variants stay guarded on both platforms by @juliusmarminge in pingdotgg/t3code#13172
* refactor(mobile): git sheets use uniwind platform variants instead of className ternaries by @juliusmarminge in pingdotgg/t3code#13185
* refactor(mobile): remaining className platform ternaries become class variants by @juliusmarminge in pingdotgg/t3code#13188
* fix(web): align provider emails without clipping by @Derpedyea in pingdotgg/t3code#13174
* perf(mobile): recycle the default v2 home list and scope the snooze minute tick by @juliusmarminge in pingdotgg/t3code#13149
* refactor(mobile): retire the legacy grouped thread list by @juliusmarminge in pingdotgg/t3code#13183
* fix(server): background PR checks spend less GitHub quota by @juliusmarminge in pingdotgg/t3code#13189
* fix(server): background PR sync reads summaries in batches by @juliusmarminge in pingdotgg/t3code#13198
* fix(server): GitHub PR lookups stop probing owner-qualified heads by @juliusmarminge in pingdotgg/t3code#13200
* chore(mobile): clear the legacy-list deletion fallout by @juliusmarminge in pingdotgg/t3code#13203

## New Contributors
* @CodyRay made their first contribution in pingdotgg/t3code#7827

**Full Changelog**: pingdotgg/t3code@v0.0.43-nightly.20260922.2123...v0.0.43-nightly.20260923.2135

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.43-nightly.20260923.2135
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant