Skip to content

fix(mobile): render the phase sidebar before the list that always wins - #157

Merged
tusharbhardwaj-bk merged 3 commits into
expbkmainfrom
t3code/congee
Sep 1, 2026
Merged

fix(mobile): render the phase sidebar before the list that always wins#157
tusharbhardwaj-bk merged 3 commits into
expbkmainfrom
t3code/congee

Conversation

@tusharbhardwaj-bk

@tusharbhardwaj-bk tusharbhardwaj-bk commented Sep 1, 2026

Copy link
Copy Markdown

The bug

The phase sidebar toggle did nothing on a phone across three attempted fixes. Root cause, finally traced instead of guessed:

HomeScreen has an early return if (threadListV2Enabled) at line 1159, and resolveThreadListV2Enabled returns true unless a device explicitly opted into the legacy list. My phase-sidebar branch sat after that return, so it was unreachable dead code. The preference wrote correctly, PhaseSidebarPane existed and compiled, and nothing could ever render it.

Moved above that return. Verified order in HomeScreen is now: empty state → phase sidebar → V2 list.

ThreadNavigationSidebarPane had the same shape of defect — it renders a thread list from two paths (props.nativeChrome and the fallback) and only the first was gated, leaving the flag half-wired on tablets. Both paths now gate.

Also fixed: the app was not reporting its build SHA

The phone reported 1.0.4 where a fork build must report 1.0.4+bk.<sha7>. That suffix is how docs/operations/bk-mobile-build.md detects client/server version skew, and losing it made every "which build is this?" question unanswerable — which is a large part of why this took three rounds to diagnose.

Cause was mine: to keep expo-constants off the import graph of tests reaching authClientMetadata, I made the manifest read a fail-soft require, which swallowed the genuine read too. Restored the static import (matching every other consumer in the app) and mocked the module in connection.test.ts instead — which is what features/cloud/publicConfig.test.ts already does.

Why none of this was caught earlier

All three failures are the same mistake: code placed somewhere plausible without tracing whether that line executes. Typecheck and unit tests cannot tell existence from reachability, and CI has no device. The only signal was running it on a phone.

Verification this time included reading the return order back out of both files rather than trusting placement.

Verification

245 tests across 26 files, vp run typecheck clean, vp fmt --check clean, fork markers pass. Re-verified after merging current expbkmain.

Model: Claude Opus 5 (1M context), harness: Claude Code in T3 Code.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

tusharbhardwaj-bk and others added 3 commits September 1, 2026 16:07
The phone reports `1.0.4` where a fork build must report `1.0.4+bk.<sha7>`.
The `+bk.` suffix is the only way to tell which commit a binary came from, and
docs/operations/bk-mobile-build.md leans on it to detect client/server version
skew — so losing it made the fork blind to exactly the failure mode that
document exists to prevent.

I caused it. To stop `expo-constants` dragging expo-modules-core onto the
import graph of tests reaching `authClientMetadata` (it reads `__DEV__` at
import time, which vitest does not define), I made the manifest read a
fail-soft `require`. That swallowed the real read too, so `bkBuildGitSha()`
returned null in a genuine Expo runtime and every build since has reported a
bare version.

The repo already had the right answer: `features/cloud/publicConfig.test.ts`
mocks expo-constants. So this restores the static import — matching every
other consumer in the app — and mocks the module in `connection.test.ts`
instead, which is where the import-graph problem actually was.

Verified: 245 tests across 26 files, `vp run typecheck` clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Third attempt at the same bug, so this time the reachability is verified
rather than assumed.

`HomeScreen` has an early return `if (threadListV2Enabled)`, and
`resolveThreadListV2Enabled` returns true unless a device opted into the
legacy list — so that return fires on every phone. My phase-sidebar branch sat
*after* it and was unreachable dead code. The toggle wrote its preference, the
pane existed, and nothing could ever render it.

Moved above that return. Verified order in HomeScreen is now: empty state →
phase sidebar → V2 list.

`ThreadNavigationSidebarPane` had the same shape of defect: it renders a thread
list from two paths (`props.nativeChrome` and the fallback) and I had gated
only the first, so the flag was half-wired on tablets too. Both paths now gate.

The pattern across all three failures is identical — I put code somewhere
plausible and never traced whether that line executes. Existence is not
reachability, and neither typecheck nor unit tests can tell the difference.

Verified: 280 tests across 31 files, `vp run typecheck` clean, lint clean, and
the return order in both files read back explicitly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size:M vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. labels Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.5 KiB 15.1 KiB
Codex Thread snapshot wire 7.2 KiB 7.3 KiB
Codex Live turn WebSocket wire 6.4 KiB 7.8 KiB
Codex Live turn WebSocket decoded 55.7 KiB 66.4 KiB
Codex Live turn messages 11 21
Claude Total thread wire 13.6 KiB 15.1 KiB
Claude Thread snapshot wire 7.2 KiB 7.3 KiB
Claude Live turn WebSocket wire 6.4 KiB 7.8 KiB
Claude Live turn WebSocket decoded 56.5 KiB 66.4 KiB
Claude Live turn messages 11 21

Baseline: unavailable · PR result: 34c96ff · 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: 110.4 KiB
  • Claude decoded thread snapshot: 111.1 KiB

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

@tusharbhardwaj-bk
tusharbhardwaj-bk merged commit 0df11f3 into expbkmain Sep 1, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 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