Skip to content

Kill the runs-aligned accept bounce at all four layers#695

Merged
FuJacob merged 1 commit into
mainfrom
fix/runs-aligned-accept-jitter
Jun 12, 2026
Merged

Kill the runs-aligned accept bounce at all four layers#695
FuJacob merged 1 commit into
mainfrom
fix/runs-aligned-accept-jitter

Conversation

@FuJacob

@FuJacob FuJacob commented Jun 12, 2026

Copy link
Copy Markdown
Owner

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 placementAmongAnchors clamps 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:

  1. Placement (resolver correctness, helps every consumer): when the parent value extends past the matched runs, the caret estimate now extends past the trailing edge by the measured per-character advance (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.
  2. Throttle: accepts (and corrections) invalidate the static-run walk cache through a new invalidateTransientCaretCaches provider hint, so Cotabby's own insert never pays the throttle's share of the staleness on top of the host's.
  3. Gate (the class-level shield): a same-line caret jump AGAINST the writing direction, with unchanged field and text, within 300ms of an accept is held as stale geometry (mirrored for RTL). Real backward moves change the preceding text and tear the session down through the reconciler; they cannot reach this gate. Forward jumps and vertical moves still re-anchor, and the window expiring keeps backward settles reachable for hosts that genuinely need one.
  4. Slide: derived overlays now slide by the host-measured advance, observedCharWidth from 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, and SuggestionAnchor as nonisolated: three Swift 6 forward-compat warnings that arrived with the responsive-lifecycle merge, restoring the zero-warning build.

Validation

xcodebuild test -project Cotabby.xcodeproj -scheme Cotabby -destination 'platform=macOS' \
  -skip-testing:CotabbyTests/FoundationModelDriftEvalTests \
  CODE_SIGNING_ALLOWED=NO CODE_SIGNING_REQUIRED=NO
# ** TEST SUCCEEDED ** 1581 tests, 0 failures, 0 crashes

# Clean build emits 0 project warnings (3 before this branch).

swiftlint lint --quiet
# 0 findings on the files this PR touches

New regression locks:

  • Placement: text grown past the last run reports the trailing gap; interior gaps near the previous edge report it; line-break gaps and oversized gaps keep the snap; in-run carets report zero.
  • Throttle: invalidate() forces a fresh walk inside the window.
  • Gate: backward word-width drift inside the accept window holds; the same drift outside the window (or with no recorded acceptance) re-anchors; forward drift and line changes re-anchor inside the window; the RTL mirror holds rightward jumps and allows leftward ones.
  • InsertedTextAdvance: the measured run char-width outranks the resolved font, which outranks nothing.
  • Coordinator rig: an accept stamps lastAcceptanceAt and invalidates the transient caret caches exactly once.

The diagnosis matches paired anchor_x stale-then-fresh log entries captured around Tab-accepted-chunk stages 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

  • The placement extrapolation changes resolver output only when the parent value extends past the matched runs, which previously produced a caret pinned to a stale edge; the extrapolated estimate is capped (64 chars, same-line) and the overlay layout still clamps to the usable frame at present time.
  • The backward-drift hold is scoped four ways: same field, same text, within 300ms of an acceptance Cotabby itself performed, and horizontal-only (vertical moves re-anchor). Its worst case is a deferred backward settle landing one window later.
  • The throttle invalidation restores at most one extra run walk per accept, the cost perf(ax): lazy Chromium descendant BFS, session-scoped invariant caches, throttled run walk, shared TextKit stack #679 was saving on idle polls, not on accepts.
  • Package.resolved note for local builds: the untracked pin must be at cotabbyinference 87193cd (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.

  • Placement layer (AXTextGeometryResolver): when the parent text value extends past matched run frames, extrapolableGapCharacters counts the short, same-line trailing gap and the resolver extends the X estimate by observedCharWidth × gap instead of parking at the stale trailing edge. Newline-spanning and oversized gaps (>64 chars) keep the snap.
  • Throttle layer (StaticTextRunWalkThrottle): new invalidate() is called through FocusTrackingModel.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.
  • Gate layer (SuggestionOverlayStabilityGate): new caretDriftDemandsReAnchor holds 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.
  • Slide layer (OverlayController): .derived-quality overlays now use observedCharWidth-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 backwardDriftHoldWindowMilliseconds and its /// block in the middle of the shouldRePresent doc comment causes the function to lose its documentation in tools like Quick Help — a cosmetic defect with no runtime impact.

SuggestionOverlayStabilityGate.swift — the backwardDriftHoldWindowMilliseconds doc comment absorbed the shouldRePresent doc comment; no other files require special attention.

Important Files Changed

Filename Overview
Cotabby/Support/SuggestionOverlayStabilityGate.swift New caretDriftDemandsReAnchor helper and backwardDriftHoldWindowMilliseconds constant 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 old shouldRePresent comment without a blank line, leaving shouldRePresent undocumented.
Cotabby/Services/Focus/AXTextGeometryResolver.swift Adds trailingGapCharacters to CaretRunPlacement and extrapolableGapCharacters helper; extrapolation is correctly gated on same-line, ≤64-char gaps; parent NSString passed through placementAmongAnchors to enable safe range extraction.
Cotabby/Support/InsertedTextAdvance.swift New overload prefers observedCharWidth (direct run-frame measurement) over font-based approximation; UTF-16 length is used consistently with how charWidth is derived, so the proportional multiplication is correct.
Cotabby/Services/Focus/StaticTextRunWalkThrottle.swift Adds invalidate() that clears all three cached fields; straightforward and correctly tested by the new test_invalidate_forcesAFreshWalkInsideTheWindow case.
Cotabby/App/Coordinators/SuggestionCoordinator+Acceptance.swift Stamps lastAcceptanceAt and calls invalidateTransientCaretCaches() on both the normal chunk-accept path and the typo-correction path, before cancelPredictionWork; ordering is correct.
Cotabby/App/Coordinators/SuggestionCoordinator+Prediction.swift Adds millisecondsSinceLastAcceptance to the shouldRePresent call (computed lazily at reconcile time) and calls invalidateTransientCaretCaches() on the auto-typo-fix path (without stamping lastAcceptanceAt, correctly, since the overlay is hidden immediately after).
Cotabby/Services/UI/OverlayController.swift Extends the advanceInline host-measured slide to .derived quality overlays and threads observedCharWidth into the new InsertedTextAdvance.width overload; falls back to the ghost-width slide when neither width source is available.
Cotabby/Models/FocusModels.swift Adds nonisolated to ResolvedFieldStyle and FocusedInputSnapshot; correct Swift 6 forward-compat annotation for Sendable value types used across async boundaries.
Cotabby/Models/SuggestionSubsystemContracts.swift Adds invalidateTransientCaretCaches() to the SuggestionFocusProviding protocol with a default no-op extension; clean protocol evolution that doesn't break existing conformers.
CotabbyTests/SuggestionOverlayStabilityGateTests.swift Six new test cases cover backward-hold, window expiry, forward-still-re-anchors, line-change bypass, RTL mirroring, and nil-acceptance no-shield; all edge cases from the PR description are exercised.
CotabbyTests/CaretRunPlacementTests.swift Five new placement tests validate trailing-gap extrapolation, interior-gap detection, newline barrier, oversized-gap refusal, and in-run zero-gap; arithmetic verified against test inputs.
CotabbyTests/InsertedTextAdvanceTests.swift New test confirms observedCharWidth outranks 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 again
Loading

Comments Outside Diff (1)

  1. Cotabby/Support/SuggestionOverlayStabilityGate.swift, line 34-65 (link)

    P2 shouldRePresent loses its doc comment after the insertion

    The new backwardDriftHoldWindowMilliseconds constant and its /// block were inserted without a blank-line break at the end of the existing shouldRePresent doc comment. In Swift, a contiguous /// block attaches to the immediately following declaration, so lines 34–64 now document backwardDriftHoldWindowMilliseconds, and shouldRePresent (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.

    Fix in Codex Fix in Claude Code

Fix All in Codex Fix All in Claude Code

Reviews (1): Last reviewed commit: "fix(overlay): kill the runs-aligned acce..." | Re-trigger Greptile

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.
@FuJacob FuJacob merged commit a26bd3e into main Jun 12, 2026
4 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.

1 participant