Skip to content

test(daemon): pin the acting route's own ambiguity issuance (ADR 0014 follow-up to #3340) - #3347

Merged
thymikee merged 2 commits into
mainfrom
fix/ambiguity-issuance-followup
Oct 9, 2026
Merged

thymikee merged 2 commits into
mainfrom
fix/ambiguity-issuance-followup

Conversation

@thymikee

@thymikee thymikee commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

Summary

Follow-up to the merged #3340, carrying the two findings its final review round confirmed as not-yet-shipped (the maintainer merged c6acd1e5e at 06:59Z before this commit landed).

  1. Acting-route coverage pin. The touch runtime's consumed-capture slot had no test of its own: deleting consumedCapture.state = snapshot (interaction-runtime.ts) degraded every press/fill ambiguity to count-only with zero red tests — the existing press test pressed an is-minted ref and traveled the selector-route seam. A new handleInteractionCommands test presses an ambiguous label= and asserts the refusal itself carries refsGeneration and a refFrameScope equal to the printed candidate bodies. Mutation-checked: removing the slot fill makes exactly this test fail.
  2. Docblock narrowing. publishAmbiguousMatchCandidateRefs claimed every AMBIGUOUS_MATCH producer runs through it; find's refusal (buildAmbiguousMatchError, AMBIGUOUS_MATCH and unsupported find actions give agents nothing to recover with #1597) prints candidates without issuing and never called it. Narrowed to the routes that consume the rule, with find's contract stated explicitly (no refsGeneration, hint routes to narrowing, no issued-ref affordance advertised). Making find's candidates issuable is a separate contract change for a maintainer to decide.

Validation

  • pnpm check:affected --run (one run) at 266d23891: all runnable checks passed.
  • The reviewer's four-step live iOS RN Catalog gate was executed at this head (runtime code identical to merged c6acd1e5e plus test/docblock changes): raw output posted on fix(is): report ambiguous selector matches as ambiguity, not absence (#2870) #3340 — ambiguous is visible 'role=text' → AMBIGUOUS_MATCH matches=22 + 5 pinned candidates (~s367781 = issued generation); plain @e2 refused (partial frame, plain_ref_requires_complete_frame); pinned @e3~s367781 tapped at (148, 30) = the listed candidate's rect center.

View guided diff Turn on auto-fix

…he rule's docblock

The review's two follow-ups, both confirmed at c6acd1e:

1) The acting seam had no test of its own. Deleting the touch runtime's
consumed-capture slot degraded every press/fill ambiguity to count-only
with no test red -- the existing press test pressed an is-minted ref,
traveling the selector-route seam. An ambiguous label= press through
handleInteractionCommands now asserts its OWN refusal carries
refsGeneration and a refFrameScope equal to the printed candidate
bodies. Mutation-checked: removing the slot fills makes exactly this
test fail.

2) The rule's docblock claimed every AMBIGUOUS_MATCH producer runs
through the helper; find's refusal (buildAmbiguousMatchError, #1597)
prints candidates without issuing them and never called it. Narrowed
to the routes that consume the rule, with the find shape named and its
contract stated: it prints no refsGeneration and its hint routes the
caller to narrow the locator, so it advertises no issued-ref
affordance. Routing find through the rule would make its candidates
issuable -- a separate contract change, left to the maintainer.
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.13 MB 5.13 MB 0 B
Package (unpacked) 5.13 MB 5.13 MB 0 B
Package (download) 1.55 MB 1.55 MB +5 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.7 ms 28.5 ms +0.8 ms
CLI --help 83.1 ms 85.9 ms +2.9 ms

@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 2 files

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

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread src/daemon/session-snapshot.ts Outdated
…claim

The review nit was right that the old phrasing leaned on an inference
("advertises no issued-ref affordance") that a reader could overread as
"find's candidates cannot be acted on" -- they are printed as ordinary
snapshot lines, and a plain ref against an unrelated complete frame is a
pre-existing admission the rule does not govern. The builder's own
message ("Use a more specific locator or selector.") and absent
refsGeneration remain the facts; state those directly and defer the
issuance question as the contract decision it is.
@thymikee

thymikee commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

check:affected recorded at head f16db6547: one full pnpm check:affected --run, all runnable checks passed. The head adds only the docblock rewording the review thread asked for (3+/3-, comment-only); the test commit 266d23891 was recorded green earlier. Review thread answered and resolved: #3347 (comment)

@thymikee

thymikee commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

This PR is ready at f16db65. The new press test pins the acting route's own ambiguity issuance, the only production change is the docblock, and its claim about the find refusal matches selector-match-errors.ts. All 19 checks are green and there are no conflicts; I reviewed by reading the code and did not run the test locally.

Not blocking: the acting-route test sits in a file named for the selector-runtime seam, while source topology would put it beside interaction-touch-runtime; a touch-runtime sibling file or a broader file name is optional.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 9, 2026
@thymikee
thymikee merged commit 8c966f3 into main Oct 9, 2026
19 checks passed
@thymikee
thymikee deleted the fix/ambiguity-issuance-followup branch October 9, 2026 18:20
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-09 18:20 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant