Repository navigation
fix(serve-sim): read observed native hinge state - #383
Conversation
🦋 Changeset detectedLatest commit: b9e3e1b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Codex implemented the native hinge readback fix, regressions, and documentation at Gabe’s direction, and ran the local checks described in the PR. Gabe has reviewed the final diff at |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Rebased onto Codex ran the native build with Xcode 26.4, lint, typecheck, LLP checks, and the isolated suite: 2,606 passed, 221 skipped, zero failures. Knip retains the same diagnostics as clean Only this PR has been pushed so far; the remaining stack updates will wait for its CI. — Codex |
0fa59e9 to
03683e0
Compare
sjkim-expo
left a comment
There was a problem hiding this comment.
when the native call fails, hingeControlEligibility() returns {} through guard(), so 0x13 responds with ok: true and only tableMode. that makes serve-sim hinge appear to succeed. could that path return ok: false instead?
LLP 0004’s serve-sim hinge paragraph also still says it prints the confirmed state and doesn’t mention tableModeActive. I couldn’t run a Duo locally, so this is code review only.
03683e0 to
9c30b67
Compare
|
@sjkim-expo Fixed in Codex updated LLP 0004’s CLI paragraph to distinguish the acknowledged — Codex |
9c30b67 to
d79c4f5
Compare
|
Codex rebased #383 onto main The rebuilt package passed 2,615 isolated tests (223 skipped, zero failures), formatting, lint, typecheck and LLP validation. Knip matches the unchanged main baseline. CI for this head is pending; upper layers remain queued. — Codex |
sjkim-expo
left a comment
There was a problem hiding this comment.
this now returns ok: false when the native hinge read fails, and LLP 0004 covers tableModeActive and failed reads. Both points are resolved; I haven’t run Duo QA locally.
brentvatne
left a comment
There was a problem hiding this comment.
Reviewed at d79c4f5 with Codex, Grok and Claude reviewers; I checked each finding below against the code.
1. Older helpers now read back the pre-command angle (medium-low)
When the helper does not advertise supportsHingeStateReadback, hinge-command.ts:135-138 returns the raw native sample instead of the config's command fields:
const { hingeAngle, hingePose, physicalOrientation, tableMode, tableModeAvailable } =
config.observedDeviceState === undefined
? config
: observedHingeState(config.observedDeviceState);Released helpers already send observedDeviceState (0160c0a), so they take this branch. After an acknowledged hinge 90, an immediate hinge.get returns the old angle: executing the reader with such a config gave hingeAngle: 0, hingePose: "closed" for an acknowledged 90. That is the failure this PR fixes for new helpers.
The test at hinge-command.test.ts:119 makes this deliberate, but the description says "Legacy helpers retain config-based readback" and LLP 0004:43 says "Screen config retains its command fields for the CLI". It matters in practice because hinge.get spawns the host's installed serve-sim against a server that may still be running the previous version, which is the normal state right after a host upgrade.
Fix: keep the config's command fields when supportsHingeStateReadback is absent, or update the description and LLP to the new behavior.
2. Readback right after an acknowledged Tent reports no pose (low-medium)
setTableMode (CoreDeviceBridge.swift:325) never calls recordHingeControl, and the eligibility overlay (CoreDeviceState.swift:121) applies commands to angle and orientation only. After Tent, readback gets angle 80 and facedown from the command but the last motion sample's tabletop activity, so observedHingePose returns null until the next motion sample arrives. The base returned "tent". The fixture's Tent test sets activity first, so it does not cover this.
3. Empty eligibility reports a confident empty state (low-medium)
The device-state subscription delivers an empty revision-0 snapshot at once, and clearObservedDeviceState also clears the command records on reconnect. Before the first sample, or after a reconnect, hinge.get returns ok: true with no angle and tableModeAvailable: false, stating unavailability that is actually unknown. LLP 0004 documents omitted fields as unknown, but not tableModeAvailable: false, and no test covers this state.
4. Tests depend on run order (low)
The new readback session tests share one fixture: the third expects the 180 left by the first, and the fourth's tableMode: false exists only because the third's angle command cleared the second's true. They also exercise a mocked eligibility, so the Swift overlay in #2 is covered only by the native tests.
Checked and fine: 0x13 is queued on hingeControlUpdate, so a read sees earlier commands and not later ones; generation and reboot checks run on both sides of the await; non-Duo devices fail before 0x13; the hinge.get output only gains tableModeActive.
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Keep legacy readback, record table controls with field timestamps, preserve unknown availability, and isolate fixture state. Co-authored-by: Codex <codex@openai.com>
d79c4f5 to
9592731
Compare
|
Thanks for checking these against the running behavior, Brent. All four findings are addressed in
Codex implemented this follow-up after Gabe approved the direction. The rebuilt CLI passed 15 immediate readbacks on an owned Duo, including Tent, plus fractional-angle checks. The full isolated suite (2,624 tests), 22 native tests, build, pinned lint and LLP checks passed. Typecheck/Knip failures match untouched main. No hosted EAS run is claimed; existing human approvals and reviewer requests are unchanged. /devin review Please review the exact current head, especially legacy helpers, Table Mode overlays and reconnect generations. — Codex |
Return reconciled controls and raw activity in one native actor read without storing another cache. Co-authored-by: Codex <noreply@openai.com>
|
Confirmed; thanks for catching the distinction. Codex added a regression where the acknowledged setting is true but native activity remains false; it failed on The follow-up at The full isolated suite (2,624 tests), 22 focused checks, 23 native tests, JS/native build, pinned lint and LLP checks passed. The rebuilt CLI also passed another 15 immediate readbacks on a fresh owned Duo, and the server/device were removed. Typecheck retains the unchanged main baseline error. Gabe approved the direction but has not inspected this follow-up. /devin review Please review this exact head again, especially raw activity vs control reconciliation and unknown samples. Human approvals and requests are unchanged. — Codex |
What
Read reconciled native hinge state after earlier queued controls. Newer external changes replace acknowledged commands;
tableModeremains the persistent setting andtableModeActivereports native activity. Legacy helpers retain config-based readback.Stack: #383 → #384 → #385 → #388.
Why
The previous readback could return the last requested pose after an external fold, or an old native angle immediately after an acknowledged command.
Authorship and LLM use
Primarily generated by Codex: code, tests, documentation, this description, and local validation. Gabe encountered issues while testing the new
simulator:controlcommand and directed stacked fixes and repeated reviews. Codex and Devin review led to ordered readback, separate setting/activity semantics, and a regression for native state changing during an awaited read. Gabe reviewed the final diff at0fa59e9b1and requested review from Krystof and Brent. No human code edits or additional human testing are claimed. Gabe owns follow-up.Gabe reviewed the final changes at
b9e3e1bf0.Evidence
CI Checks passed on
0fa59e9b1. Simulator test CI was canceled at Gabe's request because of the separate Xcode 27 stack.Lint: existing React diagnostics match the unchanged source baseline.
Required workspace build/typecheck commands: local serve-sim JS/native build passed; local typecheck retains only baseline
scripts/tart/dev.ts:61. Root workspace build commands were not rerun locally; CI Checks passed.Full local tests: 2,605 pass, 217 skip, one baseline failure (
device sessions follow the active native screen), previously reproduced on unchanged source. The last helper-retention guard was added afterward; its 71 focused helper/screenshot/middleware checks pass.Device-backed validation by Codex: Owned Duo, iOS 27.1 (24A94401), Xcode 27.1 beta: external folds read back as 180°/0°; immediate acknowledged set/get checks returned 90°/0°/180°, and Table Mode enablement returned true. The final mixed-sample refinement passed an async native-boundary regression.
Regression evidence: Red/green regressions preceded the fixes, including the mixed-sample case in
10ef77ab6.CLI/native/UI evidence:
hinge.getafter acknowledged sets:hingeAngle: 90,0,180; Table Mode:tableMode: true. Native commands and readback were exercised locally.13 focused outer tests passed; the session fixture covers stale samples, external changes, activity, and queue ordering. Xcode-blocked serve-sim CI test workflows were canceled at Gabe's request; other checks and Devin reviews were not explicitly canceled by Codex. Hosted fix validation remains planned.
Rollback
Plain revert.
— Codex
Review follow-up
Brent identified stale legacy readback, immediate Tent readback, unknown availability, and order-dependent tests. Gabe agreed with the fixes and asked to consolidate command state. Codex kept legacy config readback, added a Table Mode acknowledgement overlay, routed successful controls through one recorder, and isolated the tests. Raw observations remain separate. Gabe reviewed this follow-up as part of the final changes.
Codex validated this at
95927314a: local JS/native build, 2,624 isolated tests, 22 focused native tests, pinned lint, and LLP checks passed. On an owned iPhone Duo (iOS 27.1, Xcode 27.1 beta), 15 immediate pose readbacks and fractional-angle set/get checks passed. The server and device were removed afterward. Local typecheck and Knip retain the same failures as untouched main; hosted EAS validation was not run. All six CI checks passed on95927314a; Devin then identified the activity/setting distinction addressed below.— Codex
Activity readback follow-up
Devin found that an acknowledged Table Mode setting could appear as native activity. Codex now returns raw tabletop activity alongside reconciled controls in one native read. Immediate Tent readback remains reconciled;
tableModeActiveuses only the motion sample, including unknown activity. Gabe reviewed the final changes atb9e3e1bf0.Codex ran 2,624 isolated tests, 22 focused checks, 23 native tests, the JS/native build, pinned lint and LLP checks. All passed. Another 15 immediate readbacks passed on a fresh owned Duo; the server and device were removed. Local typecheck retains the unchanged main baseline error. All six checks, including Devin Review, passed on
b9e3e1bf0; no review threads remain open; hosted EAS validation was not run.— Codex
Hosted validation on the current stack
Codex tested the exact #385 tip
d78473fb0, including #383 atb9e3e1bf0, in an owned EAS web-preview-only session. The Xcode 26.6-built package was pinned to its CI artifact; the worker package pin and logs matched. Device: iPhone Duo, iOS 27.1, Xcode 27.1 (27A9275), UDID74195B53-4DBC-4EAA-9991-F98E5BB86BA8.All five pose readbacks, fractional angles (14.5°/53.5°), four opened-Duo CLI rotations, UI rotation, same-turn Laptop/Rotate controls, and reconnect passed. Browser screenshots/recording and advancing live video frames confirmed preview updates; a host screenshot also succeeded. The expo.dev preview worked with its session token. This run did not exercise legacy addons, injected inactive tabletop samples, or Argent recovery. The owned session was stopped after testing.
— Codex