test: finish the gh-145 acceptance coverage - #488
Closed
mberrys wants to merge 8 commits into
Closed
Conversation
Completes the test half of issue #145. The implementation landed across 12e9b91 and 711a011; these are the criteria those commits left unproven, plus the two defects writing them exposed. Interaction controller: - dragContinuesBeyondViewportEdge, with pointer capture held while the pointer is outside the viewport and released on the up (AC3); - dragSnapsPreviewOnlyAfterThreshold and snapMovesPreviewNotCommittedGeometry: a snap is presentation, the target the drag steers is untouched, and DragSession::snappedTo records the source rather than leaving a commit path to re-derive it; - snapSuppressModifierIsSampledPerMove: Alt pressed and released mid-gesture starts and stops the snap without restarting the drag, which is what AC3 means by sampling modifiers consistently; - hitTestToleranceFollowsLiveViewScale (AC5); - traceRecordsHitTestCandidatesAndPreciseHits (AC7); - staleIndexEntryCannotSurviveIntoACommit (AC6): re-indexing under a live drag is not staleness and must not cancel it, while a revision change refuses the completion. Viewport controller: zoomAnchorHoldsWithinSubpixelToleranceAcrossZoomRange pins the anchor to a declared AnchorTolerancePx of 0.5 -- one rounding of the integer offset, which is the best the arithmetic can do and what AC4 asks to be stated rather than left implicit -- and zoomAnchorHoldsOnRotatedPages covers 90/180/270 (AC4, AC8). Hit-test sources: toleranceExpandedQueryCrossesCellBoundaries is the case PageSpatialIndex::query(QRectF) exists for, and toleranceQueryReturnsSpanningItemsOnce pins the de-duplication (AC5, AC8). EditorHost: pageBoxSnapProviderOffersCornersOfTheHittableBoxes proves the provider reads the same PageBoxHitTestSource the dispatcher hit-tests, so a snap target cannot drift from a hit target, and that it offers corners only. Two defects these tests found, fixed in 12e9b91: - the screen-to-page conversion divided by the zoom alone. The zoom is half the page-to-screen scale; the other half is pixelPerMM, so on a 96 dpi display a 2 px hit tolerance reached closer to 8 px. hitTestToleranceFollowsLiveViewScale fails against the old form. - the AC7 candidate counts ran through the trace's duration formatter, reporting twelve candidates as a p50 of 0.000012 ms. Run locally against Qt 6.11.1 (mingw): UnitTestsDragSnapper 10, UnitTestsHitTestSource 16, UnitTestsViewportController 19, UnitTestsInteractionController 33, UnitTestsEditorHost 8, all passing, alongside the rest of the interaction module and scripts/verify-interaction-boundary.py. clang-format is not installed in this environment, so the format gate is hosted-CI only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eaaa76a9ae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- keep only changes/cdx-gh139-interaction-quality-closeout.md which matches the branch, remove the two extra fragments that made check-change fail - rewrite the summary in plain language without em dashes or dense AI-style lists
mberrys
pushed a commit
that referenced
this pull request
Sep 1, 2026
Six more scenarios covering the direct-manipulation half of issue #146's list: hover on a dense page, drag with and without snapping, cursor-anchored zoom, rapid zoom reversal with page switching, and a page whose overlay exceeds its bounds. hover-dense asserts the index candidate count rather than latency. A spatial index that stopped narrowing and a page that simply got heavier look the same in a frame-time percentile; only the candidate count separates them, which is the regression issue #145 can actually suffer. zoom-reversal is the corpus port of InteractionControllerTest::rapidZoomReversalAndPageSwitchSettleWithinTraceBudget. The C++ case stays where it is as the unit-level guard. A manifest entry may now carry blocked_on with a blocked_reason. drag-snap needs DragSnapper, which is still in PR #488, so nothing can run it yet. Marking it explicitly is what keeps the report coverage check strict for the other eight: without it the check would have to be relaxed for the whole corpus, and a scenario that silently stopped running would become invisible. Also documents the two lanes, the contract evaluation order, and the trace stage to phase mapping in INTERACTION_CONTRACT.md, and records Q-05 in the 0.2.0 closeout matrix as partial -- no verified latency measurement exists for this candidate, and the matrix must not imply one does. Refs #146, #139. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWGFQCBSNwStAPYUCAZzpN
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.