docs(mobile): Hermes V1 pin A/B measured no RAM reduction - #4837
Conversation
96154f1 fixed the untargeted tmux session lookup in dispatch-role.sh itself (session resolved through $TMUX_PANE, falling back to an own session), so the workaround this learning recorded is obsolete.
From e2e-verifier r0 (baseline gate): booting on an explicit port around default-serial claim contention, maestro flow traps on Android under host load (exit-code, full-string regex, ANR/timeout hardening), and recovering a wedged CLI relay window. The stale-claims cause is updated for the guest-boot-id claim keying that landed in 96154f1.
A bare session target lets tmux compute the new window's index, which can land on an occupied slot: on a two-window session with active window 0 the first dispatch after launch fails with 'create window failed: index 1 in use' (tmux 3.7b). Target "<session>:" so tmux appends at the next free index instead.
…sion Patch react-native version files and override hermes-compiler to match upstream 0.86.2's lockstep pair. Document removal at RN ≥ 0.86.2/0.87 and local native-cache bust (fingerprint ignores root patches).
…atch The original patch hunk dropped upstream's '\ No newline at end of file' marker on the + side, so the patched sdks/.hermesv1version gained a trailing newline. ReactAndroid/hermes-engine/build.gradle.kts reads that file with readText() and no trim. Re-authored via pnpm patch-commit so the patched file stays byte-identical to upstream apart from the version string; version.properties hunk unchanged. Lockfile diff is the patch hash only.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe last commit ( Overview
Issue Details (click to expand)WARNING
Files Reviewed (9 files)
(No content changes to these 9 files since the last review at Fix these issues in Kilo Cloud Previous Review Summaries (2 snapshots, latest commit 41d095c)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 41d095c)Status: No Issues Found | Recommendation: Merge Executive SummaryThe on-device A/B measurement failed the pre-committed threshold (median TOTAL PSS rose from 881.7 MB to 901.7 MB, Native Heap flat), so the Hermes V1 patch, Files Reviewed (11 files)
Previous review (commit a9aba33)Status: No Issues Found | Recommendation: Merge Executive SummaryReviewed the Hermes V1 engine/compiler version pin (patch, override, lockfile, docs) and the unrelated workflow-tooling changes; no security, correctness, or breaking-change issues found with high confidence. Files Reviewed (9 files)
Reviewed by claude-sonnet-5 · Input: 26 · Output: 11.6K · Cached: 787.8K Review guidance: REVIEW.md from base branch |
Metro file-map staleness after node_modules relayout, pnpm v11 no-op install virtual-store drift, e2e stack kills under foreign slot churn, iOS cold-build ShareExtension PODS_ROOT defect, iOS Maestro row/back idioms.
…uction Single-session on-device A/B (PR #4837 verifier r1, kilo_pixel7 emulator, debug dev-client, identical locked flow, 3 cold starts per arm, per-ABI libhermesvm.so hashes confirmed distinct): median TOTAL PSS 881.68 MB (.0.14) vs 901.65 MB (.0.16) -> +19.97 MB median Native Heap 608.69 MB (.0.14) vs 608.57 MB (.0.16) -> -0.11 MB The frozen gate (>=15 MB lower on either signal, 2/3 iterations consistent) missed on both signals, so the workaround buys nothing and must not ship (plan ship-nothing branch). Removes the react-native patch, the hermes-compiler override and the lockfile change; keeps an apps/mobile/AGENTS.md note with the measured numbers recommending the fix via React Native >= 0.86.2 / 0.87.
# Conflicts: # .kilo_workflow/dispatch-role.sh
iscekic
changed the title
|
(bot) @kilocode-bot please review the current head (639543b): the diff changed since your last summary — a merge of origin/main absorbed the dispatch-role.sh change and replaced the slot-churn learning with an update to the existing e2e-slot entry. |
Review — head
|
| File | Change |
|---|---|
apps/mobile/AGENTS.md |
+1 bullet (Hermes V1 regression, tried pin, measured medians, take-the-fix-via-RN-0.86.2 recommendation) |
.kilo_workflow/learnings/e2e-slot-release-stopped-another-sections-stack.md |
+12 (victim-side recovery recipe appended to the existing entry) |
8 × .kilo_workflow/learnings/mobile-*.md, pnpm-install-noop-*.md |
new environment learnings |
gh pr diff --name-only agrees exactly with the local three-dot diff — no drift between GitHub's view and the branch.
2. dispatch-role.sh is out, slot-churn file absorbed — confirmed
.kilo_workflow/dispatch-role.shis not in the three-dot diff, and the head's copy is byte-identical toorigin/main(git diff origin/main FETCH_HEAD -- .kilo_workflow/dispatch-role.shis empty). The branch's earlierbb0293338role-window fix is fully absorbed by main's lineage (#4826/subsequent), so this PR no longer carries shared-script changes.e2e-slot-stack-killed-by-foreign-slot-churn.mdwas added on the branch (04893205f) and removed in the merge (639543bde, −21), with its operational content folded into the existinge2e-slot-release-...entry (+12 net vs main). No orphan file, no duplicated entry, and the churn learning's substance survived rather than being silently dropped. Good resolution.
3. No product/app code remains — confirmed
- No
patches/react-native@0.86.0*.patch(only the pre-existingexpo-server-sdkandreact-native-appsflyerpatches). - No
hermes-compiler/ Hermes entry anywhere inpnpm-workspace.yaml;apps/mobile/package.jsonstill"react-native": "0.86.0"and untouched. - No lockfile change in this PR. For the record: a naive two-dot
git diff origin/main FETCH_HEAD -- pnpm-lock.yamlshows −3 lines (@kilocode/session-ingest-contracts), but that is main-side content added after the merge base by#4832, not PR content — the three-dot diff has zeropnpm-lock.yamldelta. - Zero files under
apps/,services/,packages/other than the singleAGENTS.mddoc line.
4. Learnings content — spot-checked against the tree, all claims hold
- e2e-slot entry (updated): the appended
stack=none-for-live-stacks attribution to the#4836name-mangling bug matchese5fb4f27c's own commit message verbatim ("status always printed 'stack=none' even with the stack up"), andstatusdoes gate ontmux has-session -t "$(stack_session "$wt")"(.kilo_workflow/e2e-slot.sh:171). "already fixed" is accurate on current main (#4836+#4842both merged). The new paragraph reads as a natural continuation of the existing entry — symptom → cause → fix → operational note → victim-side recovery — with no contradiction of the prose above it. mobile-android-stale-claims-explicit-port.md:96154f115(#4826, Android claims/boot-id) exists on main as described.mobile-android-maestro-flow-pitfalls.md:Search sessions...placeholder confirmed (session-list-search-header.tsx:35);open-app.yaml'sextendedWaitUntil … timeout: 30000confirmed (line 31), so the "30 s is too short" note is grounded.mobile-ios-maestro-session-row-and-back.md:accessibilityLabel={… 'Go back'}confirmed (screen-header.tsx:107).pnpm-install-noop-leaves-virtual-store-drift.md: repo is onpnpm@11.1.2, consistent with the v11 short-circuit claim; its cross-reference tomobile-metro-file-map-stale-after-node-modules-relayout.mdresolves to a file added in this same PR (no dangling link).- Numbers are internally consistent: the medians in
AGENTS.md(881.7 / 901.7 TOTAL, 608.7 / 608.6 Native) are the correct medians of the PR body's three iterations per arm, and the ship-nothing conclusion follows from the pre-registered ≥15 MB gate (TOTAL went +19.97 MB the wrong way, Native −0.11 MB).
5. Non-blocking nits (no change requested)
- The new
AGENTS.mdbullet is one 873-char line; the longest pre-existing line in that file is 230.format-checkis green, so this is purely readability — a nested sub-bullet block would be easier to scan later. mobile-android-stale-claims-explicit-port.mdandmobile-native-build-cache-ignores-root-patches.mdembed host-specific detail (/Users/igor/Projects/.worktrees/...,~/Library/Caches/Kilo/...,$TMPDIR).WORKFLOW.mdsteers machine-only facts to the gitignoredlearnings/system/. Committing these is defensible since the substance is repo-script behavior (fingerprint scope, claim files), but the literal absolute paths will read as noise on another machine.mobile-ios-cold-build-shareextension-pods-root.mdidentifies a real repo-side defect (project-levelREACT_NATIVE_PATHvia${PODS_ROOT}leaks into theShareExtensiontarget, which has no Pods xcconfig) and only records a temporary local workaround. That deserves a tracked follow-up issue for the config plugin, otherwise the next cold iOS build rediscovers it from a learning file.
6. CI / merge state
All required checks green on this head (typecheck, lint, format-check ×2, test, test (kilo-app), check-unused, drizzle-check, markdown table padding, CodeQL, trufflehog); mergeStateStatus: BLOCKED is solely REVIEW_REQUIRED. The unrunnable local pnpm test (web/web-env) caveat in the description is moot — the diff contains no web/service code and CI ran those suites green.
Recommendation
Approve / merge. Dropping the pin is the right call: a workaround that misses its own pre-registered threshold (and trended the wrong way on TOTAL PSS) should not ship, and the residual value — the measurement, the "don't retry this" record, and the RN ≥ 0.86.2 path — is exactly what belongs in AGENTS.md plus learnings. The diff is now clean of shared-script and dependency churn.
|
The re-review is already posted: #4837 (comment) Verdict: docs-only, ship-nothing diff confirmed after the merge — dispatch-role.sh absorbed cleanly, slot-churn content folded into the existing e2e-slot entry with no orphan/duplicate, no product code or lockfile changes snuck in. A few non-blocking nits (long AGENTS.md line, host-specific paths, suggest tracking the ShareExtension |
|
(bot) Re-review on 639543b acknowledged — thanks. On the non-blocking nits: (1) the AGENTS.md bullet follows the file's existing one-line-bullet style (the bullet it replaced was the same shape); (2) host-specific paths in learnings are by design — they are machine-local environment records; (3) the ShareExtension REACT_NATIVE_PATH fix candidate is recorded in .kilo_workflow/learnings/mobile-ios-cold-build-shareextension-pods-root.md for whoever owns that config plugin; a separate tracking issue is out of this section's scope. No code changes from the nits. |
2 participants
Summary
What this PR became: a documentation note and workflow learnings — no product change
ships. The section implemented a Hermes V1
250829098.0.16pin forreact-native@0.86.0to remove the per-worklet ~512 KB metadata regression of the bundled
.0.14engine, measuredit against the unmodified engine in a single on-device A/B round, and found no measurable
RAM reduction — so the patch, override and lockfile change were dropped per the plan's
pre-registered ship-nothing branch. A workaround that measurably buys nothing must not ship.
What remains in the diff:
apps/mobile/AGENTS.md— one bullet recording the regression, the tried pin, the measurednumbers below, and the recommendation to take the fix via React Native ≥ 0.86.2 / 0.87
(plus the standing prohibition of worklets Bundle Mode while NativeWind remaps
react-native, reanimated#9817)..kilo_workflow/learnings/— eight new environment learnings from the two device rounds,plus a victim-side recovery recipe added to the existing slot-release entry.
Why the pin was expected to help: RN 0.86.0 ships Hermes V1
250829098.0.14, whichattaches ~512 KB of metadata per unique worklet
(Software Mansion;
fixed in
.0.15+). This app importsreact-native-reanimatedin ~57 source files, so ~30live worklets would exceed the 15 MB gate. RN 0.86.2 itself carries the fix but is blocked by
the repo's
minimumReleaseAgegate until ~2026-08-01 and pinned by Expo SDK 57 — the upgraderemains the correct path once eligible.
Verification
On-device A/B (the decision evidence). One emulator boot (
kilo_pixel7, debugdev-client), one locked flow (login → Agents → session transcript → Profile → KiloClaw),
three cold-start iterations per arm; arm A
.0.14vs arm B.0.16, engine swap proven byper-ABI
libhermesvm.sosha256 (all four ABIs differ across arms) and the Gradle cachegaining
250829098.0.16;dumpsys meminfoApp SummaryPss(KB):Frozen thresholds (≥ 15 MB lower on either median, 2/3 iterations consistent): both
signals missed — TOTAL PSS went +19.97 MB the other way (0/3 below), Native Heap moved
−0.11 MB (noise). Arm A reproduced the pre-merge baseline within ~1.6% (895.71/607.60 MB),
so the measurement environment was sound. Behavior on the
.0.16engine was otherwisecorrect on both platforms: Android cold launch/login/lists/animations clean with zero
error-class Hermes/worklets logcat lines; iOS
Podfile.lockresolvedhermes-engine (250829098.0.16)and the smoke flow passed — the pin is safe, it just buys nothing here.Automated gates (green locally): root
pnpm format:check+pnpm typecheck;apps/mobile:format:check,typecheck,lint,check:unused,test(277 files /2337 tests),
pnpx expo-doctor20/20. Rootpnpm test(web/web-env) could not be run greenlocally under host saturation (load 275–740; 1303×60 s timeouts, 330×5 s, 11 connection
terminations on shared postgres — all in
apps/websuites, none touching this diff, whichcontains no web code); CI on
origin/mainwas green at 2026-07-28T16:02Z, so PR CI is theauthority for those suites.
Visual Changes
N/A — no user-facing change remains in the diff.
Reviewer Notes
AGENTS.mdbullet; everything else is.kilo_workflow/learnings, which ship with the run per repo convention. The branch is current with
origin/main(merged 2026-07-28 ~21:45, including thee2e-slotfixes fix(workflow): stop e2e-slot mangling the stack session name #4836/fix(workflow): stop e2e-slot release from stopping another section's stack #4842 and thehuman-readygate rule).target's
REACT_NATIVE_PATHexpands an emptyPODS_ROOT); it is tree-independent,present with or without the pin, and recorded in
.kilo_workflow/learnings/mobile-ios-cold-build-shareextension-pods-root.mdwith a properfix candidate.
e2e-slot.sh releasebug fixed infix(workflow): stop e2e-slot release from stopping another section's stack #4842 (killed twice mid-round; measurements already taken stayed valid and only failed
iterations were re-run). The recovery recipe is added to
.kilo_workflow/learnings/e2e-slot-release-stopped-another-sections-stack.md.