Skip to content

fix(decision): count failed actions as no progress in the stall guard - #844

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

okwasniewski merged 1 commit into
tester-army:mainfrom
pvedula7:fix/decision-stall-clock

Conversation

@pvedula7

@pvedula7 pvedula7 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

What and why

  • The decision executor blocks a step after three actions in a row that make no progress. stalled() in packages/decision/src/executor.ts only read pageChanged, so on a screen with a ticking timer every observation looked new, and a run of failed actions never tripped it.
  • An entry now counts as no progress when its action errored or the page did not change. An action that ran without an error and changed the page still breaks the run. The blocked summary now reads "Three actions in a row failed or changed nothing on screen."
  • The page fingerprint is untouched, so a time picker going from 10:30 to 10:45 still counts as progress.
  • Any error counts, not only the same action failing with the same error. The repro's failures were identical, so both rules catch it, and three failures in a row of any kind is no progress either.
  • On main, a skipped pick and a rejected done or failed claim are history entries with an error too, so they count toward the three as well. Before, they counted only when the page had not changed. The changeset says so.
  • Two existing tests asserted the old summary (the bottom-of-page scroll test in executor.test.ts and the no-vision half of the point-tap test in executor-vision.test.ts); they now expect the new one.

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. On the new layout the timer test carries the port: it fails against main's executor.ts and passes here.
  • packages/decision/tests/executor.test.ts, blocks three failed actions while a timer ticks: every tap fails with APP_UNREACHABLE while a clock node ticks each observation.
  • 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. After typing, a number pad covers "Log set" and every tap on "Finish" fails with APP_UNREACHABLE while the rest timer ticks.
main this branch
timer test 12 failed taps, then "The step did not conclude within its 25 decision calls." blocked after 3 failed taps: "Three actions in a row failed or changed nothing on screen."
client-workout, live 5 and 8 failed "Finish" taps, each recorded as a page change. Jev then claims failed and the check confirms it, so the run reports ACTION_FAILED, an app failure that isn't one 2/2 runs, 4/4 attempts stop blocked after 3 failed taps: "Three actions in a row failed or changed nothing on screen." All three still read as a page change, so the errors alone tripped the guard

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 4 files

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

Re-trigger cubic

Comment thread packages/decision/src/elements.ts Outdated
@okwasniewski

Copy link
Copy Markdown
Member

Thanks for the detailed repro, it pinpoints a real bug, but we'd like to fix it differently. Masking clock-shaped text means any real change that only touches such text (a time picker going 10:30 to 10:45, a score, a duration) reads as "nothing changed", and the regex duplicates CLOCK_PATTERN in observation.ts.

In your repro every "Finish" tap failed with APP_UNREACHABLE. The executor records that error in history (executor.ts:229), but stalled() (executor.ts:323) only looks at pageChanged, so the ticking clock hid the failures. Could you change stalled() to count an errored action as no progress (e.g. N errored actions in a row, or the same action failing with the same error) and drop the masking? Your timer test makes a good regression test for that.

@pvedula7 pvedula7 changed the title fix(decision): ignore ticking clocks when deciding a page changed fix(decision): count failed actions as no progress in the stall guard Oct 5, 2026
@pvedula7

pvedula7 commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Makes sense, I've dropped the masking (672e778). stalled() now counts an entry as no progress when its action errored or the page didn't change, so three of either in a row blocks the step (e2d372c). The timer test now has every tap fail with APP_UNREACHABLE: it runs out the 25-call budget on main and blocks after three taps here.

@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 fix is still wanted: stalled() in executor.ts still only counts unchanged pages. Could you rebase onto main and reapply it?

It's the same one-line change in stalled() plus the blocked message. Two existing tests assert the old message ("Three actions in a row changed nothing on screen."), so update those. One thing to know: on main, history entries also carry an error when a pick is skipped and when a done or failed claim is rejected, so those now count as no progress too. That's fine by us, but please keep a test for the ticking-timer case.

stalled() looked only at pageChanged, so a timer ticking on screen made
every observation look new and hid a run of failed actions. In the repro
every "Finish" tap failed with APP_UNREACHABLE while the rest timer
ticked, and the model kept tapping until it claimed the step failed.

An entry now stalls when it errored or left the page unchanged, and the
blocked message says so. Any action that ran without an error and changed
the page still breaks the run of three. Skipped picks and rejected done or
failed claims also record an error in history, so they count too.
@pvedula7
pvedula7 force-pushed the fix/decision-stall-clock branch from f1c7584 to dfd2cb9 Compare October 9, 2026 19:31
@pvedula7

pvedula7 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main and reapplied: the stalled() line, the new blocked message, and the two tests that asserted the old one (in executor.test.ts and executor-vision.test.ts). The ticking-timer test fails against main's executor.ts (12 failed taps, then the decision-call budget runs out) and passes here, and the changeset now notes that skipped picks and rejected done or failed claims count too. pnpm check and pnpm test pass.

@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 5e55d7c into tester-army:main Oct 9, 2026
23 checks passed
@github-actions github-actions Bot mentioned this pull request Oct 9, 2026
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