Key ghost font stabilizer on field identity, not focus sequence#432
Merged
Conversation
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
Ghost text was doubling in size in chat composers like Slack and iMessage when the input field grew to a second line. The
GhostFontSizeStabilizeralready clamps caret height to the smallest reading observed during a focus session, but it was keyed onfocusChangeSequence, whichFocusTrackerbumps whenever the focused-input polling signature changes — and that signature includes the field'sinputFrameRect. So as soon as the field grew taller (text wrapping onto line 2), the stabilizer treated it as a brand-new focus session, lost its minimum, and the first post-wrap caret reading (often the coarse field-height fallback) became the new baseline.Add a stable
focusedInputIdentityKeyderived from(bundleIdentifier, processIdentifier, elementIdentifier)onFocusedInputContext, plumb it throughSuggestionOverlayGeometry, and key the stabilizer on it instead. The key survives self-resizing inputs but still resets on genuine field switches.focusChangeSequencekeeps its existing meaning for other consumers (e.g.,VisualContextCoordinator).Validation
Manual verification (chat composer wrap) is best done with
-cotabby-debugrunning on real hardware; I haven't done that in this session.Linked issues
None.
Risk / rollout notes
Hasher, which is randomized per process. That's fine here — the key is only ever compared within one process's lifetime.elementIdentifieris CFHash-derived and can theoretically collide across genuinely different fields in the same process. In that pathological case the stabilizer would carry the previous field's minimum into the new field, capping the new field's ghost font slightly small. The downstreamminimumGhostFontSizefloor bounds how small it can get, so the failure mode is "ghost text a touch smaller than ideal" rather than the current "ghost text doubles." Net trade is strongly positive.focusChangeSequenceis unchanged; other consumers that depend on it (notablyVisualContextCoordinator) are unaffected.Greptile Summary
Fixes ghost-text doubling in self-resizing chat composers (Slack, iMessage) by keying
GhostFontSizeStabilizeron a newfocusedInputIdentityKey— derived from(bundleIdentifier, processIdentifier, elementIdentifier)— instead offocusChangeSequence, which was inadvertently reset whenever a growing input field changed its frame rect.FocusedInputContextgains a computedfocusedInputIdentityKey: UInt64that omits the input frame from its hash, so the key is stable across field growth but distinct across genuine field switches.SuggestionOverlayGeometrycarries the new key as a stored property (defaulting to 0 for tests), andSuggestionCoordinator+Acceptance.swiftthreads it through to the geometry at construction time.GhostFontSizeStabilizer.swift(not in this diff) still describes the session key as "FocusTracker'sfocusChangeSequence" — it should be updated to referencefocusedInputIdentityKeyto stay accurate.Confidence Score: 4/5
The change is targeted and well-reasoned; the only loose end is a stale doc comment.
The logic change is confined to a single call site in
showInline, the new identity key correctly excludes the frame rect that was causing spurious session resets, and the fallback behavior (a slightly-too-small ghost font on hash collision) is clearly bounded. The doc comment onfocusChangeSequenceinSuggestionOverlayGeometrynow incorrectly claimsOverlayControllerkeys stabilization on it, which could mislead future maintainers. The parallel stale comment inGhostFontSizeStabilizer.swift(outside the diff) compounds this slightly.The
focusChangeSequencedoc comment inSuggestionModels.swiftneeds updating;GhostFontSizeStabilizer.swift(not in this diff) has a matching stale comment worth cleaning up in a follow-up.Important Files Changed
focusedInputIdentityKeycomputed property onFocusedInputContext(hashing bundle ID, PID, element ID) and a matching stored property onSuggestionOverlayGeometry; doc comment on the existingfocusChangeSequencefield is now stale.showInline: passesfocusedInputIdentityKeyinstead offocusChangeSequenceas thefocusSessionKeyto the ghost-font stabilizer; change is isolated and well-commented.focusedInputIdentityKeyfromFocusedInputContextintoSuggestionOverlayGeometry; no logic change.Sequence Diagram
sequenceDiagram participant FT as FocusTracker participant FIC as FocusedInputContext participant SC as SuggestionCoordinator participant SOG as SuggestionOverlayGeometry participant OC as OverlayController participant GFSS as GhostFontSizeStabilizer FT->>FIC: snapshot (bumps focusChangeSequence on frame change) FIC->>FIC: compute focusedInputIdentityKey\n(bundleId + pid + elementId, no frame) FIC->>SC: context with focusedInputIdentityKey SC->>SOG: build geometry(focusChangeSequence, focusedInputIdentityKey) SOG->>OC: geometry OC->>GFSS: stabilizedCaretHeight(caretHeight, focusSessionKey: focusedInputIdentityKey) GFSS-->>OC: clamped height (session min survives field growth) OC->>OC: resolvedGhostFontSize → render inline ghost textComments Outside Diff (1)
Cotabby/Models/SuggestionModels.swift, line 445-448 (link)focusChangeSequencedoc comment still says "OverlayController keys its per-session font-size stabilization on this value" — that claim is now false. After this PR,OverlayControllerdrives stabilization fromfocusedInputIdentityKey;focusChangeSequenceis consumed by other paths (e.g.,VisualContextCoordinator). Leaving the stale claim will mislead the next reader about which key actually governs ghost-font session scoping.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (1): Last reviewed commit: "Key ghost font stabilizer on field ident..." | Re-trigger Greptile