Skip to content

fix(decision): forward an element's selected state to the model - #910

Merged
okwasniewski merged 4 commits into
tester-army:mainfrom
Ashfaqbs:fix/decision-forward-selected-state
Oct 12, 2026
Merged

okwasniewski merged 4 commits into
tester-army:mainfrom
Ashfaqbs:fix/decision-forward-selected-state

Conversation

@Ashfaqbs

@Ashfaqbs Ashfaqbs commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #884.

checked and expanded already reach the decision model through the element table (actionSpace) and target criteria (targetCriterion), but selected did not, even though the engine already reports it in SemanticNode.states. On a tab bar every tab therefore looked identical to the model, so:

  • the completion check for a tab switch landed around 50/50 (holds/inconclusive), sometimes blocking a step that actually worked
  • a step could be marked done before anything was tapped, because the model couldn't tell the already-active tab from the one it needed to reach

Change

Forward selected the same way checked already is:

  • packages/decision/src/elements.ts: add selected?: boolean to Element, copy row.node.states?.selected into the element table
  • packages/decision/src/questions.ts: include selected in both elementRecords and targetCriterion

No operation logic changes — tab was already tappable; this only forwards state that was already being read by the engine but dropped before it reached the model.

Test plan

  • elements.test.ts: new test asserting a tab's selected state appears in the element table
  • executor.test.ts: new test asserting selected appears in tap_target criteria, covering the same path checked already has a test for in check_target criteria
  • pnpm --filter @e2e-dev/decision test: 75 passed (the 4 pre-existing cli.test.ts skips are unrelated — that suite needs Node ≥22.22.3, and this sandbox has 22.22.0)
  • pnpm --filter @e2e-dev/decision typecheck: clean
  • oxlint on the changed files: clean

Added a patch changeset.

Review in cubic

checked and expanded already reach the decision model through the
element table and target criteria; selected did not, even though the
engine already reports it in SemanticNode.states. On a tab bar every
tab therefore looked identical, so the completion check for a tab
switch landed near 50/50: a step that worked could still be blocked,
and a step could be marked done before anything was tapped.

Forward selected the same way checked already is, in actionSpace's
element table and in targetCriterion.

@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.

No issues found across 5 files

Re-trigger cubic

@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: elements carry checked and expanded but not selected. Could you rebase onto main and port it?

In the new layout:

  • elements.ts: add selected to Element and set it from row.node.states?.selected next to checked and expanded. elementRecords spreads the element, so the state picks it up without changes.
  • questions.ts: target options are prose now (targetCriterion), so append selected there next to checked/unchecked when it is true.

…d-selected-state

# Conflicts:
#	packages/decision/src/elements.ts
#	packages/decision/src/questions.ts
@Ashfaqbs

Copy link
Copy Markdown
Contributor Author

Merged main and ported the change as you described:

  • elements.ts: selected added to Element, set from row.node.states?.selected next to checked/expanded; operations now uses the filtered variable introduced by feat(decision): full action grammar, none fallback, and vision through the OpenAI Decisions API #965 instead of raw row.operations.
  • questions.ts: targetCriterion is prose now, not a record — added if (element.selected === true) parts.push('selected') next to the checked/unchecked line. elementRecords already spreads the element, so no change needed there.
  • executor.test.ts: the existing "shows selected state in tap target criteria" test still asserted the old record-shaped criteria and a nonexistent tap_target question key; updated it to match the prose format the checked test next to it already uses (requests[1].questions['target']).

Decision package typecheck, oxlint, and vitest all green; full pnpm check passes too (confirmed the two pre-existing dead-code/override findings it reports are unrelated — identical on a clean upstream/main checkout).

@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 added this pull request to the merge queue Oct 12, 2026
Merged via the queue into tester-army:main with commit 42ce272 Oct 12, 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.

decision: selected state never reaches the model, so tab steps are a coin flip

2 participants