Trust native AX caret geometry; scope the layout-estimate override to web content

[FuJacob]

Summary

#670's caret repair gave the hidden-text-layout estimate authority to override a .derived AX rect whenever the two disagreed vertically by more than 0.75 line, on the theory that a mismatch means the host mapped the caret into the wrong visual line. That theory is correct for web engines' AX bridges (the Gmail drift it was built for) and wrong for native apps: Notes answers previous-character AXBoundsForRange from its real layout (a 23pt title line above 16pt body lines, plus paragraph spacing), which a uniform hidden layout cannot model, so within a few lines the layout "disagrees" while AX is exactly right, and the override moved ghost text a full line off the caret. This PR scopes estimator authority by geometry provenance: a new WebContentFieldDetector classifies the focused field as web-rendered (DOM-reflection attributes on the resolved element, or a known browser/Electron bundle), .derived geometry in native hosts now bypasses the estimator unconditionally (restoring pre-#670 anchoring there), and .estimated substitution stays universal because AXFrame-only hosts have no real measurement to protect.

Detection adds no AX traffic (the resolver already fetches the attribute-name list it keys on), unknown hosts default to native (worst case is pre-#670 behavior, never a new failure), and the bypass now logs a skipped outcome with a skip_reason, which also closes the silent run-measured path flagged in #670's review.

Validation

Live AX probe of Notes on this machine (macOS 26), which pinned the regression mechanically before the fix was designed:

• The body AXTextArea vends no DOM-reflection attributes and exposes no static-text child runs, so it resolves through previous-character bounds: .derived without run frames, exactly the mode Estimate caret position from a hidden text layout when AX exposes only the field frame #670's override applies to.
• Per-line AXBoundsForRange shows mixed line boxes (title 23pt, separator 17pt, body 16pt) that the estimator's pinned uniform line height cannot reproduce; the cumulative error crosses the 0.75-line override threshold within about three lines.
• Zero-length AXBoundsForRange in Notes is systematically one visual line high as well as zero-width, so Branch 1's empty-rect rejection is load-bearing and was deliberately left untouched.

xcodebuild -project Cotabby.xcodeproj -scheme Cotabby -destination 'platform=macOS' build \
  -derivedDataPath build/DerivedData
# ** BUILD SUCCEEDED **

xcodebuild test ... -skip-testing:CotabbyTests/FoundationModelDriftEvalTests \
  CODE_SIGNING_ALLOWED=NO CODE_SIGNING_REQUIRED=NO
# ** TEST SUCCEEDED ** full suite, 1062 tests, 0 failures
# (SuggestionCaretLayoutRepairTests: 11 tests, including the new native-derived
#  keep-AX lock and the native-estimated still-substitutes lock;
#  WebContentFieldDetectorTests: 11 tests covering the DOM-attribute/bundle matrix)

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

The repair stage now emits repair_outcome: "skipped" with skip_reason: "native_host_geometry" | "run_measured_geometry", so the trust decision is auditable per request_id under -cotabby-debug.

Not yet verified end-to-end in a live Notes typing session; the evidence is the probe-derived geometry plus the unit locks above.

Linked issues

Refs #670. The Notes-class regression was reported directly after #670 merged and has no separate issue filed.

Risk / rollout notes

• The behavior change is confined to .derived geometry in non-web hosts: they return to pre-Estimate caret position from a hidden text layout when AX exposes only the field frame #670 anchoring (the AX rect, rendered inline as before). Web hosts (DOM-attribute elements, browser bundles, Electron editors) keep the Estimate caret position from a hidden text layout when AX exposes only the field frame #670 override and the run-measured bypass bit-for-bit, so the Gmail/Outlook wins are unchanged.
• Misclassification is asymmetric by design: an unrecognized web host merely degrades to pre-Estimate caret position from a hidden text layout when AX exposes only the field frame #670 behavior, while a native host can only be exposed to the override if it vends DOM-reflection attributes, which native AppKit elements never do.
• Browser-chrome fields that are not DOM-backed (e.g. the omnibox) stay estimator-eligible via the bundle signal; these are single-line fields where the vertical-agreement test short-circuits, and their toolkit's AX is browser-grade rather than AppKit-grade anyway.
• FocusedInputSnapshot/FocusedInputContext gain one immutable Bool; equality-based polling dedupe is unaffected because the flag is constant for a given focused field.
• project.pbxproj regenerated by XcodeGen (additions only).

Greptile Summary

This PR scopes the #670 caret-layout-repair estimator to web-rendered fields only, fixing a regression where native apps like Apple Notes (which use their own layout manager for rich-text geometry) had ghost text shifted off the real caret line. A new WebContentFieldDetector classifies each focused field as web or native using two independent signals: DOM-reflection attributes already present in the fetched AX attribute list (no extra round-trip), and a bundle-identifier check via BrowserAppDetector.

• New WebContentFieldDetector classifies fields as web vs. native, with unknown hosts defaulting conservatively to native (worst case: pre-Estimate caret position from a hidden text layout when AX exposes only the field frame #670 behavior, never a new failure mode).
• layoutRepairedAnchor gating now bypasses the estimator for all .derived geometry in native hosts; web hosts retain the prior line-mismatch override path; .estimated geometry (no real measurement exists) still substitutes regardless of host type.
• Structured logging adds repair_outcome: \"skipped\" with a skip_reason enum, closing the previously silent run-measured and native-host bypass paths that were unreachable from the log stream.

Confidence Score: 4/5

Safe to merge; the behavioral change is isolated to .derived geometry in non-web hosts, which revert to pre-#670 anchoring. Web hosts are unchanged bit-for-bit.

The fix is well-scoped and mechanically verified against the Notes regression with live AX probes. The two style notes are non-blocking: a redundant nil-guard in the logging path and the reuse of BrowserAppDetector.isBrowser (a tone-hint utility) to drive the trust policy, which is correct today but creates a latent coupling risk if that prefix list is extended for unrelated reasons in the future.

WebContentFieldDetector.swift and its use of BrowserAppDetector.isBrowser are worth a second look to confirm the coupling is acceptable long-term; all other files are straightforward propagation or test additions.

Important Files Changed

Filename	Overview
Cotabby/Support/WebContentFieldDetector.swift	New enum with two static classification signals (DOM-reflection attributes + bundle identifier). Logic is clean and well-documented; the minor concern is that isWebContentField delegates to BrowserAppDetector.isBrowser, which was designed for a different purpose and could silently widen the trust policy if its prefix list is extended for tone-hint reasons.
Cotabby/App/Coordinators/SuggestionCoordinator+Acceptance.swift	Core fix: native .derived geometry now bypasses the estimator unconditionally, web .derived geometry retains the prior line-mismatch gate, .estimated path is unchanged. The logLayoutRepairDiagnostic update has a redundant if let skipReason check that's always non-nil at that branch. Removal of kept_ax_run_measured log value is correct (that path was previously unreachable).
Cotabby/Services/Focus/FocusSnapshotResolver.swift	Correctly wires vendsDOMAttributes through AXFocusCandidate to snapshot assembly without adding any new AX round-trips; the attribute name list is already fetched earlier in the same candidate probe.
Cotabby/Models/FocusModels.swift	Adds isWebContentField: Bool with a false default that keeps all existing call sites compiling unchanged; participates in Equatable synthesis harmlessly since the flag is constant for a given focused element.
Cotabby/Models/SuggestionModels.swift	Propagates isWebContentField from FocusedInputSnapshot into FocusedInputContext correctly.
CotabbyTests/WebContentFieldDetectorTests.swift	New test file covering DOM-attribute signal, bundle-only signal (Chrome, Safari, VS Code, Firefox omnibox path), nil-bundle default, embedded WKWebView in native app, and the unknown-bundle-defaults-to-native invariant. Good coverage of the classification matrix.
CotabbyTests/SuggestionCaretLayoutRepairTests.swift	Adds isWebContentField: true to existing derived-geometry tests (preserving their intent) and new native-host tests locking the Notes-class regression and the native-estimated-still-substitutes invariant.
CotabbyTests/CotabbyTestFixtures.swift	Adds isWebContentField: Bool = false to both fixture builders; default value preserves existing test behavior.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[layoutRepairedAnchor called] --> B{quality == .estimated or .derived?}
    B -- No --> C[Return: rect as-is, outcome: nil]
    B -- Yes --> D{quality == .derived?}
    D -- No .estimated --> E[Run layout estimator]
    D -- Yes --> F{isWebContentField?}
    F -- No native host --> G[Return: AX rect, skipReason: nativeHostGeometry]
    F -- Yes web host --> H{observedContentEdges != nil?}
    H -- Yes --> I[Return: AX rect, skipReason: runMeasuredGeometry]
    H -- No --> E
    E --> J{Estimator result}
    J -- .rejected --> K[Return: fallback rect]
    J -- .estimate --> L{verticallyAgrees?}
    L -- Yes --> M[Return: AX rect, log: kept_ax_agreement]
    L -- No --> N[Return: estimate rect, quality: .layoutEstimated]

Loading

Comments Outside Diff (2)

1. Cotabby/App/Coordinators/SuggestionCoordinator+Acceptance.swift, line 809-822 (link)

   Redundant nil-check on skipReason

   The outer guard anchor.outcome != nil || anchor.skipReason != nil else { return } ensures that when execution reaches this inner guard let outcome else-branch (i.e., outcome == nil), skipReason is guaranteed non-nil. The if let skipReason nested inside will always bind, which can mislead a future reader into thinking there is a reachable code path where skipReason could be nil at that point. A force-unwrap with a comment, or restructuring as let skipReason = anchor.skipReason!, would make the invariant explicit.

   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!

   

2. Cotabby/Support/WebContentFieldDetector.swift, line 397-406 (link)

   isBrowser repurposed outside its documented scope

   BrowserAppDetector.isBrowser is documented as the "broad 'is the user typing in a web browser' tone hint," and its prefix list is maintained with that purpose in mind. WebContentFieldDetector now gives it a second role: deciding whether AX-derived geometry may be overridden by the layout estimator. If a future contributor adds a non-web-engine bundle prefix to browserBundlePrefixes for tone-hint breadth (e.g. an Electron email client that shouldn't expose its AX geometry to the estimator), the trust policy would silently widen. A narrowly scoped isWebEngine(bundleIdentifier:) method on BrowserAppDetector, or a shared private constant the two detectors both reference, would make the coupling explicit and enforce the distinction at the call-site.

   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: "fix(caret): only let the layout estimate..." | Re-trigger Greptile}