Kill the runs-aligned accept bounce at all four layers#695
Merged
Conversation
Child-run (runs-aligned) derived hosts still bounced the ghost left then right on every word accept: Chromium publishes the inserted VALUE tens of milliseconds before the child-run AXFrames reflow, and the #679 run-walk throttle stretches that staleness up to another 100ms. The post-publish poll therefore maps the new caret offset against pre-insert runs, the placement clamps it to the stale trailing edge (a full word left), the publish sentinel has already cleared, and the 6pt drift gate re-anchors onto the accepted word before the fresh walk snaps it back. Measured bounces of 22-88px with 8-100ms gaps per accepted chunk. Four coordinated changes, one per layer: - Placement: when the parent value extends past the matched runs (text published, frames not yet reflowed), the caret estimate extends past the trailing edge by the measured per-character advance instead of parking on it, gated on a same-line gap of credible length. Fresh presents (next suggestion, anchor-cache restores) get a near-correct caret during the reflow window too. - Throttle: accepts invalidate the static-run walk cache, so Cotabby's own insert never pays the throttle's share of the staleness on top of the host's. - Gate: a same-line caret jump AGAINST the writing direction with unchanged field and text inside 300ms of an accept is held as stale geometry (mirrored for RTL); forward jumps and line changes still re-anchor, and the window expiring keeps backward settles reachable for hosts that need them. - Slide: derived overlays now slide by the host-measured advance (observedCharWidth from the same run frames, then the resolved font), so the held anchor matches the fresh-walk caret within tolerance and the legitimate settle has nothing left to move. Also annotates FocusedInputSnapshot, ResolvedFieldStyle, and SuggestionAnchor nonisolated (three Swift 6 forward-compat warnings that arrived with the responsive-lifecycle merge), keeping the build at zero project warnings.
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.
Summary
Third and final surface of the accept jitter: child-run derived hosts (
caret=derived primary (runs-aligned), Gmail-class Chromium editors) still bounced the ghost left then right on every word accept, with measured amplitudes of 22-88px and 8-100ms gaps in the dev logs.The mechanism survives both prior fixes because the staleness here outlives the text publish: Chromium publishes the inserted VALUE tens of milliseconds before the child-run AXFrames reflow, and #679's run-walk throttle (keyed only on focus sequence + time, caching run texts AND frames) stretches that by up to another 100ms. The post-publish poll, the same tick that clears #690's publish sentinel, maps the new caret offset against pre-insert runs; the inserted word exists in no cached run, so
placementAmongAnchorsclamps the caret to the stale trailing edge, a full word LEFT. Text is unchanged, the drift gate's 6pt tolerance reads the word-width gap as a genuine move, and the overlay re-anchors onto the accepted word; the fresh walk then snaps it back RIGHT. (#691 does not apply: these overlays render as.derived, not.layoutEstimated.)Four coordinated changes, one per layer of the failure:
CaretRunPlacement.trailingGapCharacters), gated on a same-line gap of credible length (a gap containing a line break keeps the snap; a huge gap refuses extrapolation). This is what protects FRESH presents too: the next suggestion after a final accept, and Responsive lifecycle: anchor reuse cache + speculative post-acceptance prefetch #689's anchor-cache restores, bypass the stability gate entirely and used to land a word left during the reflow window.invalidateTransientCaretCachesprovider hint, so Cotabby's own insert never pays the throttle's share of the staleness on top of the host's.observedCharWidthfrom the very run frames the resolver measures, then the resolved field font, so the held anchor matches the fresh-walk caret within tolerance and the legitimate settle has nothing left to move.Net accept cycle on a runs-aligned host: one anchored advance at the keystroke, hold through the publish window (#690), stale-frame ticks either compute a near-correct caret (1, 2) or are refused as backward staleness (3), and the fresh walk lands within tolerance of the slid anchor (4). Nothing moves after the accept.
Also annotates
FocusedInputSnapshot,ResolvedFieldStyle, andSuggestionAnchorasnonisolated: three Swift 6 forward-compat warnings that arrived with the responsive-lifecycle merge, restoring the zero-warning build.Validation
New regression locks:
invalidate()forces a fresh walk inside the window.InsertedTextAdvance: the measured run char-width outranks the resolved font, which outranks nothing.lastAcceptanceAtand invalidates the transient caret caches exactly once.The diagnosis matches paired
anchor_xstale-then-fresh log entries captured aroundTab-accepted-chunkstages in a live Chrome repro; re-verifying in the same field after this merge is the remaining field check.Linked issues
Refs #690, #691, #679, #689. Reported directly: "when it's runs-aligned derived, the suggestion still jitters as you accept it".
Risk / rollout notes
Package.resolvednote for local builds: the untracked pin must be at cotabbyinference87193cd(Explore streaming or chunk-based autocomplete generation #11) or newer; an older pin no longer builds main (pre-existing footgun, CI resolves fresh and is unaffected).Greptile Summary
This PR fixes accept-time overlay jitter on "runs-aligned derived" hosts (Gmail-class Chromium editors) at four coordinated layers: caret extrapolation past stale run frames, throttle cache invalidation on each synthetic insert, a directional backward-drift hold in the stability gate, and extending the host-measured advance slide to
.derived-quality overlays.AXTextGeometryResolver): when the parent text value extends past matched run frames,extrapolableGapCharacterscounts the short, same-line trailing gap and the resolver extends the X estimate byobservedCharWidth × gapinstead of parking at the stale trailing edge. Newline-spanning and oversized gaps (>64 chars) keep the snap.StaticTextRunWalkThrottle): newinvalidate()is called throughFocusTrackingModel.invalidateTransientCaretCaches()on every synthetic insert, removing up to one 100ms throttle window of stale-run reuse on top of the host's own reflow lag.SuggestionOverlayStabilityGate): newcaretDriftDemandsReAnchorholds same-line backward (against-writing-direction) jumps for 300ms after an acceptance; vertical moves, forward jumps, and expired-window backward settles still re-anchor; RTL mirrored.OverlayController):.derived-quality overlays now useobservedCharWidth-based advance (or resolved-font fallback) rather than the ghost-font width, so the held anchor matches the fresh-walk caret within tolerance.Confidence Score: 4/5
The change is safe to merge; all four layers are independently guarded and have tight regression tests. The only cosmetic defect is a misplaced doc comment block.
The four-layer fix is well-structured: each layer has clear invariants and dedicated unit tests (1 581 tests, 0 failures per CI). The backward-drift hold is carefully scoped by field identity, text identity, directionality, and a 300ms expiry so worst-case is one deferred backward settle, not a permanent block. The placement extrapolation is capped at 64 chars with a newline barrier, bounding the overshoot risk. The only issue found is that inserting
backwardDriftHoldWindowMillisecondsand its///block in the middle of theshouldRePresentdoc comment causes the function to lose its documentation in tools like Quick Help — a cosmetic defect with no runtime impact.SuggestionOverlayStabilityGate.swift — the
backwardDriftHoldWindowMillisecondsdoc comment absorbed theshouldRePresentdoc comment; no other files require special attention.Important Files Changed
caretDriftDemandsReAnchorhelper andbackwardDriftHoldWindowMillisecondsconstant implement the directional backward-drift hold; logic is correct (all four Boolean cases verified, RTL mirroring confirmed by tests). The constant's///doc block was appended to the oldshouldRePresentcomment without a blank line, leavingshouldRePresentundocumented.trailingGapCharacterstoCaretRunPlacementandextrapolableGapCharactershelper; extrapolation is correctly gated on same-line, ≤64-char gaps;parentNSString passed throughplacementAmongAnchorsto enable safe range extraction.observedCharWidth(direct run-frame measurement) over font-based approximation; UTF-16 length is used consistently with howcharWidthis derived, so the proportional multiplication is correct.invalidate()that clears all three cached fields; straightforward and correctly tested by the newtest_invalidate_forcesAFreshWalkInsideTheWindowcase.lastAcceptanceAtand callsinvalidateTransientCaretCaches()on both the normal chunk-accept path and the typo-correction path, beforecancelPredictionWork; ordering is correct.millisecondsSinceLastAcceptanceto theshouldRePresentcall (computed lazily at reconcile time) and callsinvalidateTransientCaretCaches()on the auto-typo-fix path (without stampinglastAcceptanceAt, correctly, since the overlay is hidden immediately after).advanceInlinehost-measured slide to.derivedquality overlays and threadsobservedCharWidthinto the newInsertedTextAdvance.widthoverload; falls back to the ghost-width slide when neither width source is available.nonisolatedtoResolvedFieldStyleandFocusedInputSnapshot; correct Swift 6 forward-compat annotation for Sendable value types used across async boundaries.invalidateTransientCaretCaches()to theSuggestionFocusProvidingprotocol with a default no-op extension; clean protocol evolution that doesn't break existing conformers.observedCharWidthoutranks the resolved font and that nil/empty inputs still return nil; consistent with the overload contract.Sequence Diagram
sequenceDiagram participant User participant Coordinator as SuggestionCoordinator participant FocusModel as FocusTrackingModel participant Throttle as StaticRunWalkThrottle participant Gate as StabilityGate participant Overlay as OverlayController User->>Coordinator: Tab (accept chunk) Coordinator->>Coordinator: "lastAcceptanceAt = Date()" Coordinator->>FocusModel: invalidateTransientCaretCaches() FocusModel->>Throttle: invalidate() Note over Throttle: Drop stale pre-insert run cache Coordinator->>Overlay: advanceInline(inserted, observedCharWidth) Note over Overlay: Slide by host-measured advance (.derived path) loop AX poll ticks (within ~300ms) Coordinator->>Gate: shouldRePresent(millisecondsSinceLastAcceptance: N) alt Backward drift + inside hold window Gate-->>Coordinator: false (hold — stale frame geometry) else Forward drift or vertical move Gate-->>Coordinator: true (re-anchor) end Note over Throttle: First tick pays fresh run walk Throttle-->>Coordinator: fresh run frames + trailingGapCharacters Note over Coordinator: extrapolated caretX ≈ truth end Note over Gate: Window expires at 300ms Gate-->>Coordinator: backward settle allowed againComments Outside Diff (1)
Cotabby/Support/SuggestionOverlayStabilityGate.swift, line 34-65 (link)shouldRePresentloses its doc comment after the insertionThe new
backwardDriftHoldWindowMillisecondsconstant and its///block were inserted without a blank-line break at the end of the existingshouldRePresentdoc comment. In Swift, a contiguous///block attaches to the immediately following declaration, so lines 34–64 now documentbackwardDriftHoldWindowMilliseconds, andshouldRePresent(line 93) is left with no documentation. A doc-comment viewer (Quick Help, generated docs) will show the combined block on the constant and nothing on the function.Reviews (1): Last reviewed commit: "fix(overlay): kill the runs-aligned acce..." | Re-trigger Greptile