Skip to content

refactor(daemon): relocate the find tests by what they assert - #3693

Merged
thymikee merged 7 commits into
mainfrom
refactor/daemon-find-tests
Oct 16, 2026
Merged

thymikee merged 7 commits into
mainfrom
refactor/daemon-find-tests

Conversation

@thymikee

@thymikee thymikee commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

Summary

Part 1 of 3 of the find/settle batch of the daemon test redesign (no test exports of any kind): the three find-* root tests under src/__tests__/daemon-composition/interaction/ that drove the find handler with the real command surface. Daemon halves (a) run against two new unexported package fakes; assertions about what the real src/commands/interaction/** code does (c) moved beside that code. Parts 2 (settle, disclosure, capture) and 3 (target evidence) follow one at a time. The original 15-commit branch is about 1,970 added lines, so it is split by dependency to stay reviewable (this part: ~510 added).

New package fixtures, both new files, nothing shared edited: support/command-surface-override-fake.ts (a surface whose named members are replaced per case) and support/legacy-capture-interactor-resolution.ts (installs an InteractorResolution that routes legacy snapshot capture through the shared capture double).

a / c split and test-count parity

Before = exact count at the old path (vitest JSON reporter, origin/main); after = run count per destination on this head.

Source (before) Daemon half (a) Command half (c)
find-ambiguous-match (6) execution/interaction/internal/find-ambiguous-match.test.ts (6; the two list cases run on a listing member and keep ref issuance and the partial ref frame) src/commands/interaction/runtime/selector-read-find-list.test.ts (2: "list returns every match", "unique match still lists")
find-production-route (4) execution/interaction/internal/find-production-route.test.ts (2: the surface receives preresolvedTarget for click and for fill) src/commands/interaction/runtime/interactions-find-carried-target.test.ts (4: press/fill act on the carried node; a diverged stored tree cannot retarget)
find-read-only (4) execution/selector-runtime-find-read-only.test.ts (4) covered by selector-read.test.ts (below)

14 tests before; 18 after (+4: the command halves of the two splits).

One assertion had been dropped and is restored: the sparse find-wait hint. The old root test checked details.hint against /snapshot quality verdict is sparse/; the hint is command-authored (sparseSelectorSnapshotError in selector-read-utils.ts), and the daemon test now runs on a fake surface that writes its own error. The command-side test selector-read.test.ts "runtime find wait reports sparse snapshot verdicts on the selector-read route" now asserts the message, details.reason and details.hint. Mutation: deleting the hint line from sparseSelectorSnapshotError fails exactly that test (1/20 across the file). The daemon-side sparse test no longer asserts command-authored message or reason (its fake wrote them); it keeps the daemon-owned part: the refusal passes through as COMMAND_FAILED and the stored tree is untouched.

One assertion was strengthened: "handleFindCommands wait captures fresh snapshots while polling" asserted >= 2 dispatches because the real surface owned the poll count; the fake owns it now, so it asserts exactly 3. The other command-authored assertions of the three source files are accounted for: the ambiguous-match details.matches/candidates stay in the daemon test (daemon-authored in selector-match-errors.ts), the list data and ref-frame assertions moved with the split to the daemon test and selector-read-find-list.test.ts, and the dispatched point (310, 510) is asserted command-side in interactions-find-carried-target.test.ts (SAVE_CENTER).

vi.mock

No vi.mock in any destination file; the only vi.fn spies are local to the production-route test's fake surface members, whose recorded calls are asserted (preresolvedTarget forwarding).

Mutation checks

  • Replacing find's internal: { findResolvedTarget: preresolvedTarget(match) } with internal: {} fails find-production-route "find: stop chaining two resolution engines on mutating actions #1654 production route: find click carries its resolved node" (1/2).
  • Replacing forceFresh: needsFreshSnapshot with forceFresh: false in selector-runtime-backend.ts fails "handleFindCommands wait captures fresh snapshots while polling" (1/10 across the find tests).

Selection

All destinations match packages/*/src/**/*.test.ts or src/**/*.test.ts, so unit-core selects them (pnpm test:unit, test:coverage); the sources were already in unit-core. check:affected --run ran the vitest related leg and passes.

Subpath exports

Package Before After
daemon 81 81

No subpath is freed here: the ./testing/* entries these tests used are still imported by parts 2 and 3, which delete them. No new subpath, barrel or export *.

Notes

  • Colocation (R84): the command tests are named for modules in src/commands/interaction/runtime/ (interactions-find-carried-target for interactions.ts, selector-read-find-list for selector-read.ts).
  • The command halves are new files, not appends to another worker's tests.

Validation

check:quick, check:layering, check:fallow, check:gate-manifest and check:affected --run pass on this head.

🤖 Generated with Claude Code

View guided diff Turn on auto-fix

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

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

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.29 MB 5.29 MB 0 B
Package (unpacked) 5.29 MB 5.29 MB 0 B
Package (download) 1.59 MB 1.59 MB -9 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 21.0 ms 21.9 ms +0.9 ms
CLI --help 54.9 ms 54.8 ms -0.1 ms

@thymikee

Copy link
Copy Markdown
Member Author

The move itself looks right, but one assertion from the deleted root test is now lost: nothing at 17ec921 checks the sparse find-wait hint, and CI is green (16/16) with no conflicts.

The old root test checked that details.hint contains "snapshot quality verdict is sparse". That hint comes from selector-read-utils.ts:39. The relocated daemon test in selector-runtime-find-read-only.test.ts:176 runs on pollingFindSurface, which writes its own error with no hint, so that check was dropped. The command-side test the PR names (selector-read.test.ts:231) checks only the message, the reason and the unchanged snapshot. find.test.ts:246 covers a different producer. If someone changes or deletes that hint, no test fails, and agents lose the recovery guidance on a sparse find wait. The rule: every assertion about text or details written by src/commands code must live in a command-side test. Please add a check that details.hint matches /snapshot quality verdict is sparse/ to the sparse test at selector-read.test.ts:231. Then re-check the other two split files for any assertion on command-authored text or details that was dropped instead of moved.

Not blocking, and you can take or leave these: interactions-find-carried-target.test.ts copies its fixtures from interactions-preresolved-target.test.ts and tests press, which the find route never calls, so the two fill cases could go into the existing file; legacy-capture-interactor-resolution.ts repeats installInteractorSnapshotResolution, which with mockInteractorSnapshot.mockImplementation(captureSnapshotThroughLegacyDispatchFixture) does the same job; recordingTouchSurface in find-production-route.test.ts returns a hard-coded point, while touchCreateCommandSurface would keep the (310,510) dispatched-point check; and the sparse daemon test's message and reason checks only test pollingFindSurface, and its "stores no tree" name holds only if the daemon capture path can write the tree.

Could the production-route half use touchCreateCommandSurface, with commandSurfaceWithMembers kept only for the selectors.find override and used as the base that touchCreateCommandSurface composes from? That would drop about 120 added lines and two new fixture files, and every piece already exists on main.

I did not run the suites or your mutation checks. I also did not trace whether the daemon capture runtime ever writes a sparse capture to the session, or whether the two ambiguous-match click cases on fakeCreateCommandSurface reach a surface member.

Once the hint assertion is in the command-side test, this is ready to merge.

@thymikee

Copy link
Copy Markdown
Member Author

Confirmed and fixed in 3312002. selector-read.test.ts checked only the message and details.reason, so the hint was unpinned. The sparse test now also asserts details.hint matches /snapshot quality verdict is sparse/. Mutation: removing the hint from sparseSelectorSnapshotError fails that test (1/20 in the file); restored. I re-checked the other two split files for command-authored text or details: find-ambiguous-match asserts details.matches and candidates, which the daemon authors (selector-match-errors.ts) and which stay in the daemon test, and its list data moved to the daemon test plus selector-read-find-list.test.ts; find-production-route asserted no command text, and its dispatched point (310, 510) is pinned command-side in interactions-find-carried-target.test.ts. In the daemon sparse test I dropped the message and reason checks, which only exercised the fake's own error, and renamed it to what it still pins. The PR body no longer claims that no assertion was dropped; it names this one. Not-blocking items: I left the fixture duplication and the hard-coded point as is for now; say if you want the production-route half rebuilt on touchCreateCommandSurface.

@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 (changes from recent commits).

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

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

Comment thread packages/daemon/src/execution/selector-runtime-find-read-only.test.ts Outdated
@thymikee

Copy link
Copy Markdown
Member Author

The sparse-tree test now asserts details.reason 'sparse tree' again, with the code and an unchanged-session check, and the test has a new name, so the earlier gap at 2b38ce1 is fixed. The relocated find tests are ready on code, and all 16 checks pass. The delta touches only two test files, so no production route is involved. I did not run the test suites or the reported mutation checks (hint removed gives 1/20 failing, non-sparse verdict gives 1/4 failing), so that evidence is the author's own and still needs the maintainer to accept it. The fixture-duplication and hard-coded dispatched-point notes from the earlier review are unchanged, and I did not re-evaluate them. No conflicts. Nothing else blocks merge once the maintainer accepts.

Could the new fakes reuse what command-surface-fake.ts already has? One option is to export forwardingCreateCommandSurface's captureIntoSession and use it in place of the new captureAndStoreSnapshot. Another is to have the legacy-capture helper call installInteractorSnapshotResolution and point mockInteractorSnapshot at captureSnapshotThroughLegacyDispatchFixture. Daemon test support would then keep one interactor-resolution installer and one capture-and-store routine.

The Cubic thread on the sparse test (#3693 (comment)) is fixed at this head, so you can resolve it. No other open review threads apply.

@thymikee
thymikee added this pull request to stack #3703 October 16, 2026 05:50
@thymikee

Copy link
Copy Markdown
Member Author

Accepting the gap at 2b38ce1 as fixed (sparse test asserts code, details.reason and the unchanged session). On the non-blocking notes: reusing captureIntoSession for captureAndStoreSnapshot, and routing the legacy-capture helper through installInteractorSnapshotResolution, are fake-consolidation changes. They are left out of this PR so its head and CI stay stable. The touch-fake fold already agreed for #3698 lands as a follow-up right after the stack merges, and the capture-and-store reuse can ride along with it if it fits.

@thymikee

Copy link
Copy Markdown
Member Author

Thanks. With the gap at 2b38ce1 accepted as fixed, this PR has no open blockers: the code is ready, all checks pass, and there are no conflicts. Moving the fake consolidation to the follow-up after the stack merges is fine.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 16, 2026
thymikee and others added 7 commits October 16, 2026 09:53
…gacy-capture interactor resolution

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…mand cases land beside the selector read (c)

find-ambiguous-match: 4 daemon ambiguity/--first cases (a) run on the inert surface; the 2 list
cases (a) keep ref issuance and the partial ref frame on a listing surface member, and their
command-side halves (list returns every match, unique included) are new (c) tests in
selector-read-find-list.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…a) and command-side carried-node action (c)

find-production-route: the two producer cases (a) assert the node find selected reaches the
command surface as preresolvedTarget through the real interaction handler; the four cases that
asserted the dispatched point (including the diverged-tree invariants) are command behavior (c)
and land as preresolved-target tests beside the interaction runtime.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… runtime (a)

find-read-only: all four cases are daemon decisions (forced-fresh polling, Android freshness
recovery, sparse capture storing nothing, recording deferral) and run on a polling find member over
the daemon's real capture backend. The command-side wait/sparse behavior is already pinned by
selector-read.test.ts (runtime find wait ...).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ed-target command test for its module (R84)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…p the daemon test to what the daemon decides

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eout cannot pass for it

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thymikee
thymikee force-pushed the refactor/daemon-find-tests branch from 2b38ce1 to e984462 Compare October 16, 2026 08:47
@thymikee
thymikee merged commit 0f12068 into main Oct 16, 2026
16 checks passed
@thymikee
thymikee deleted the refactor/daemon-find-tests branch October 16, 2026 08:58
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-16 08:58 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