Repository navigation
feat(limrun): share Android text and performance reads - #4268
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 20 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
|
Thanks for this. At 7ba110a the binding reuses the shared Android text reader and perf owners through the session ADB provider, which is the right shape, but it still needs a live Limrun Android run that is tied to it. The recorded run (text, frames, memory sample, HPROF) belongs to be921a1, and no ref reaches that commit after the force-push, so the claim that only the capability annotation changed cannot be checked. The new production wiring in runtime.ts:237 has no automated coverage either, so the live run is the only proof of that route. After the coverage and thread fixes, please rerun the live script at the final head and record that SHA. The run must reach The Coverage check fails because it is the changed-line gate: 14/21 changed lines are covered (66.67%) against a 70% threshold. This is likely related to this PR, since every uncovered line is added here: android-perf-runtime.ts lines 21 and 23, interaction-operations.ts line 150, and runtime.ts lines 238-240 and 907. The fix is tests, not a CI change. One provider-scenario test that opens a Limrun Android session on the existing provider fixtures and runs Not blocking, and fine to take or leave: (1) interaction-operations.ts:144 writes its own missing-interactor resolver, while the contracts package already owns that shape in providerInteractorSource and every sibling binder uses it. Could a bindProviderElementTextRuntime in contracts, built on it like bindProviderFocusInteractor, replace this for all providers? (2) The abort signal never reaches the perf adb commands in android-perf-runtime.ts:21. Local Android behaves the same, so a signal through AndroidPerfHost in platform-android would be a separate change. (3) app-log-runtime.ts:212 and interaction-operations.ts:143 gate on device.platform === 'android' while the facts are decided separately. Local Android binds from the fact with whenAdmitted, so binding perfFrames and gating text on the facts would keep one source. The open inline threads that still apply are the lease-loss error class (r4492404766) and the two coverage threads (r4492404748, r4492404758). One lower-priority thread also still applies: r4492404780, a cheap throwIfAborted after bind. Please fix the coverage and lease-loss threads first, then attach the live run at the resulting head. |
|
Addressed in 99be568. Both lease-loss and cancellation regressions fail on 7ba110a and pass here. The new provider scenario allocates a Limrun session, opens Settings, and runs memory sampling through daemon dispatch, the production runtime binding, and the session ADB executor. It checks the tunnel serial and parsed metric. Frame/HPROF forwarding and lost-lease text reads are covered too. The full coverage suite passes, changed-line coverage is 26/26 (100%), and the exact-commit affected gate passes. I also took the shared text-binder and facts-admission suggestions. Cancellation of already-running ADB performance commands remains separate, as discussed. The replacement live evidence is from this exact final head. The get-text debug trace records an Android helper capture and zero fallback events; frame and memory samples are available; the 33,515,185-byte HPROF has a JAVA PROFILE header. All test resources were cleaned up. The PR body now links this run instead of relying on the orphaned commit. |
|
This PR is ready at 99be568. It fixes what the earlier review raised: the changed-line coverage gate now passes, because the new provider scenario covers Your live run at 99be568 covers the route: the get-text value equals the node label, so the silent fallback would return the same text, but the Not blocking, take or leave: the delta adds a third inline copy of the "get the interactor or throw DEVICE_NOT_FOUND" resolver at interaction-operations.ts:142, next to the two in |
|
Addressed both optional suggestions in The new catalog regression fails at |
|
At 3848d11 the earlier clean review still holds. The new commit routes every Limrun interactor resolution through one helper, so a lost lease now reports |
Summary
Limrun Android snapshots were available, but live text and performance reads were refused. This 23-file change binds the shared text reader and Android performance operations to the session's ADB tunnel. It enables frame statistics, memory samples, and downloaded HPROF snapshots with local parsing and artifact handling. One owner resolver now reports lost sessions consistently across text, lifecycle, runner, and catalog operations. Cancellation during binding prevents work from starting.
This follows #4225. #4275 adds instance lifecycle commands separately. Native profiling remains deferred.
Examples:
agent-device get text @e1,agent-device perf frames sample,agent-device perf memory snapshot --kind android-hprof.Validation
At
3848d114b36d6de352ee20827fbb8e4cda9996c1,pnpm check:affected --run --head HEADpassed withexactCommit: true. Receipt, regression tests, and live results. The catalog lost-lease regression fails on99be56862and passes here. Independent review found no issues.A fresh Limrun Android run verified native helper text without fallback, frame/memory samples, and a 33,521,774-byte HPROF. All resources were cleaned up. Full coverage and 100% changed-line coverage previously passed at
99be56862; GitHub owns coverage and device checks for the new head.Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.