Skip to content

fix(decision): offer dismiss_keyboard while an on-screen keyboard is showing - #843

Merged
okwasniewski merged 1 commit into
tester-army:mainfrom
pvedula7:fix/decision-dismiss-keyboard
Oct 9, 2026
Merged

okwasniewski merged 1 commit into
tester-army:mainfrom
pvedula7:fix/decision-dismiss-keyboard

Conversation

@pvedula7

@pvedula7 pvedula7 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

What and why

  • The decision executor offers scroll_up, scroll_down and back next to the element operations, but not the runner's dismissKeyboard. On a device the keyboard that typing raises can cover the next control, and the tree drops whatever it covers, so the model has nothing that reaches it.
  • packages/decision/src/elements.ts now adds dismiss_keyboard to the controls when the target declares dismissKeyboard and the tree lists a keyboard or key node outside any hidden subtree. That is the probe keyboardShowing uses in packages/mobile/src/surface.ts. An iOS number pad lists only its keys. perform in packages/decision/src/dispatch.ts runs it through ctx.actions.dismissKeyboard(), so the runner authorizes, budgets and records it, and CONTROL_LABELS in executor.ts names it "dismiss the keyboard" in turns and history. Web declares no dismiss and is unchanged.
  • The criterion in packages/decision/src/questions.ts states the fact: "The on-screen keyboard is up. Controls it covers are missing from the elements; hide it to reach them." With the built-in tool's wording ("when it covers what you need to reach") Jev never picked it in 23 decisions, p at most 0.20.

Verified

Ran it locally: yes

  • The live runs below were on the layout before feat(decision): full action grammar, none fallback, and vision through the OpenAI Decisions API #965. The port onto the new layout is covered by unit tests: elements.test.ts (offered while keys show, not for a hidden keyboard, a hidden subtree, no keyboard, or no dismissKeyboard verb) and executor.test.ts (the operation question lists it and choosing it calls dismissKeyboard).
  • Our React Native app on an iOS 26.5 simulator, not the mobile benchmark: e2e run tests/client-workout.e2e.ts --no-cache with Jev 1.13 through @ai-sdk/typesafe-ai. The step types reps and weight into number-pad fields, then taps "Log set", which the pad covers.
main this branch
"Log set" is gone from the tree. Jev taps "Finish" 8 times, each APP_UNREACHABLE, then claims failed and the check agrees (ACTION_FAILED) 2/2 runs pick dismiss_keyboard, after 3 and after 7 failed taps, then tap "Log set". The logger moves to set 2
  • Its probability climbs from about 0.2 to 0.3 with each failed tap and wins once it passes tap. It sits close to tap, so how many taps it takes varies between runs.
  • Neither run passes. Run 1 ends blocked on the completion check. Run 2 types into set 2 as well, the pad comes back, and Jev taps "Finish" 9 more times before it claims failed. That loop is what fix(decision): count failed actions as no progress in the stall guard #844's stall guard is for.

Checklist

  • Changeset added (pnpm changeset) for any change to a published package, or the change is testbed, docs, or CI only.
  • Breaking change: title carries !, the changeset body names what breaks and what replaces it, and the deprecation warning landed a window earlier (stability policy in CONTRIBUTING.md).
  • Docs updated in the same PR: the docs/**/*.mdx page for the behavior, and skills/e2e/ if the skill describes it.
  • Contract change: emitted .d.ts reviewed, tests/types/sdk-types.ts updated; wire change edits the schema and both fixtures.
  • Engine contract change (e2e/engine): changesets for e2e, @e2e-dev/web, and @e2e-dev/mobile.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 6 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/decision/src/elements.ts
Comment thread packages/decision/src/questions.ts Outdated
Comment thread docs/decision-models.mdx Outdated
@okwasniewski

Copy link
Copy Markdown
Member

Thanks, good catch, and the numbers in the description make the case. One nit: please drop the JSDoc block above OPERATIONS in packages/decision/src/questions.ts; it describes a single entry and repeats the PR description.

For later: the mobile engine already has a measured keyboard state (raw.keyboard.kind in packages/mobile/src/surface.ts) that never reaches ExecutorObservation. Passing it through would make detection independent of whether the keyboard shows up in the tree (likely relevant on Android). Not blocking for this PR.

@pvedula7

pvedula7 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor Author

Thanks. The comment block above OPERATIONS is gone in 5b585d5. Agreed on the keyboard state. Will do in a follow up PR since it touches the engine contract.

@pvedula7

pvedula7 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor Author

Opened the keyboard-state follow-up as #883. On Androids agent-device only measured the keyboard on iOS, and Gboard's tree nodes don't use keyboard/key roles, so also opened callstack/agent-device#3240 to report. #883 will use that once it ships.

@pvedula7

pvedula7 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor Author

@okwasniewski
Hey, not sure what the process here is but this one should be ready for human review.

Appreciate your work on this repo!

@okwasniewski

Copy link
Copy Markdown
Member

Thanks! I merged main into this branch and renamed scriptedEvaluation to scriptedDecision in the new test, since #916 renamed the helper on main. Decision typecheck and tests pass.

@okwasniewski

Copy link
Copy Markdown
Member

Thanks for this. #965 just landed and restructured the decision package, so this conflicts now. The change is still wanted: main has no keyboard control yet. Could you rebase onto main and port it?

In the new layout it touches:

  • elements.ts: add dismiss_keyboard to Control, and add it to the controls set in actionSpace when verbs.has('dismissKeyboard') and the tree shows a keyboard (your keyboardShowing helper).
  • dispatch.ts: a dismiss_keyboard case in perform that calls actions.dismissKeyboard(). That is now the only place that touches ctx.actions.
  • questions.ts: the OPERATIONS entry. executor.ts: a CONTROL_LABELS entry.

It comes to about 15 lines, and decision typecheck and tests pass with it.

…showing

The decision executor's only non-element controls were scroll and back, so
once typing raised a keyboard that covered the next control (an iOS number
pad over a submit button), the decision model had nothing to pick that
could reach it. The runner already exposes dismissKeyboard and the built-in
agent offers it.

dismiss_keyboard is offered when the engine declares the verb and the tree
lists a keyboard or key node, the same probe the mobile engine uses. The
probe stops at hidden subtrees, so a hidden container whose keys carry no
hidden state of their own does not offer it. It dispatches through
perform in dispatch.ts, so the runner authorizes, budgets, and records it
like any other action.

Its criterion states that the keyboard is up and covers controls. Measured
with Jev on an iOS number pad: worded like the built-in tool ("hide the
keyboard when it covers what you need to reach"), Jev never picked it in 23
decisions (p at most 0.20); worded as the fact, it picked it after three
failed taps in both runs and then reached the covered button.
@pvedula7
pvedula7 force-pushed the fix/decision-dismiss-keyboard branch from 74f003e to 916453d Compare October 9, 2026 19:31
@pvedula7

pvedula7 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main and ported to the #965 layout as one commit: dismiss_keyboard joins Control and the controls set in elements.ts (tree probe unchanged), perform in dispatch.ts calls actions.dismissKeyboard(), and questions.ts and executor.ts get the criterion and the CONTROL_LABELS entry. The tests, the docs line and the changeset came along, and decision typecheck and tests pass, as do pnpm check and pnpm test.

@okwasniewski okwasniewski left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@okwasniewski
okwasniewski merged commit ac657d1 into tester-army:main Oct 9, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants