[PoC, do not merge] push-based auto-refresh on accessibility-tree changes - #1006
[PoC, do not merge] push-based auto-refresh on accessibility-tree changes #1006gabrielecirulli wants to merge 6 commits into
Conversation
|
@y3owk1n worth taking for a spin if you're interested. Much less flaky after the last few commits and there's also no flicker during updates. One issue is some of the hints change labels during their lifecycle (or appear to due to refresh). It's not able to keep track of their identity. For what it's worth, the same would happen if you refreshed manually, I think. I think this should probably be possible somehow. |
i havent actually tested it out manually but just looking through the code briefly, and ya, this is what i meant that it will need lots of different notification sources to sort of guess if the layout actually changes, and it seems like that's the only way to get a more reliable content drifts notification. This PR is definitely huge, but what i see is that it includes several group of changes that worth to be split out:
The one big important thing is that, how should we architect this properly in a more declarative and modular way. If you notice most of our recent commits, we try to stay away from defining new configuration keys due to the fact that configuration controls the whole program and it's less flexible. Potentially patterns like any of the following:
I have no idea yet what are the best for now (need more thinking), but the goal is that if possible, we do not want to impose changes in config but find a way to make them modular so that users that are interest in this, can build the command out. I also see that you have added some sort of stable ID and reusing labels across hints refreshes, i am not sure if we need to add these tho, unless the performance is the issue, or else, i think we can just rebuild the whole tree as it is and let the generator do it's thing, so that we don't need to have some magic diffing that is hard to debug later on. I think the label changes across refreshes is expected, we don't need to spend time to ensure their consistency across refreshes. If possible, I don't think we should touch anything on hints and element related code in Overall, amazing work and it proves that this is something that can be work out. The rest are just more on architectural design, ux design and how can we implement this with as little code changes as possible, which requires more planning and thinking. One last thing that worth for a deep thought is that, is auto refresh worth the hassle of all of these, when it's just 1 key bind away to refresh the hints manually. For this, i can't really provide any opinion, as I don't daily drive hints at all anymore. |
|
Thanks! To be fair, it took a while to get Claude set up with context and a plan, so I didn't end up reviewing the code it wrote. I'm not that familiar with the codebase yet, so I'll have to take a deeper dive before restructuring this. At a high level I agree with you:
It is just a keystroke away, but for me it significantly cuts the mental effort of using the tool, since the labels are always where you need them. |
…acOS)
Opt-in auto-refresh of hints while hints mode is active, driven by AXObserver
notifications instead of a fixed post-action delay. Observers run only during
hints mode (zero idle cost) and watch exactly the processes the scan targeted.
- native AXObserver bridge on a dedicated, lazily started run-loop thread;
arm/disarm/release are marshalled onto that thread so an observer is never
released while its callback is dispatching
- ObserverManager actor: generation-tagged reconciles so teardown cannot be
lost or undone, epoch stale-guard, self-pid exclusion, ref-counted thread
- observation targets resolved from the scan (strategy-aware: vision emits no
front-window target)
- RefreshCoordinator: debounce + mid-typing defer + max-defer floor
- stable hint labels via CFHash identity and prefix-free label reuse
- restructured Electron enablement: listless AXManualAccessibility for all
apps, AXEnhancedUserInterface gated by additional_ax_support.escalate_enhanced,
patient/TTL cache so native apps are not re-probed each activation
- config: hints.auto_refresh.{enabled,debounce_ms,watch_value_changed}
- debug logging of AX notifications to diagnose refresh triggers
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Field testing of the push-based hint auto-refresh surfaced a feedback loop and gaps in change detection. This commit addresses them. Changes: - Suppress observer-driven refreshes while neru's own scan is running, plus a short margin after it. Reading the accessibility tree makes some apps churn their own elements (post AXCreated/AXUIElementDestroyed), which fed straight back into another refresh and made hints flicker continuously after the first selection. A scanning flag set for the whole scan window breaks that loop. - Drop kAXFocusedUIElementChanged from the front-window mask. It fires on mere focus/hover shuffles and is not needed to detect that the hintable set changed, so it only added churn. - Add AXLoadComplete to the front-window mask so browser page navigation triggers a refresh once the new document finishes loading. - Raise the auto-refresh debounce default from 80ms to 150ms, doubling as a short settle delay so a scan reads the tree after a burst of changes lands rather than mid-change. Notes: - Auto-refresh remains opt-in and off by default. - Known gap: a real change whose notification lands inside the self-scan suppression window is dropped, so roughly one refresh in five misses fresh elements. The proper fix is a post-scan content fingerprint (element count + bounding-box hash) so self-induced churn can be told apart from a real change without a lossy time window; that lets the suppression window go away. Not in this PoC. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The push auto-refresh muted observer refreshes for a fixed margin after every scan, to swallow the create/destroy notifications a scan induces in some apps (the flicker loop). That margin was lossy: a real change whose notification landed inside it was dropped, and because the change is often still settling, its settled state was never re-scanned. Roughly one refresh in five missed the fresh hints, and adding delay could not help because the notification was discarded regardless of timing. Each scan now fingerprints the hint set it produced (order-independent, from each element's stable identity and bounds). The post-scan margin opens only when the fingerprint is unchanged, meaning the scan saw nothing but its own churn. A scan that changed the set caught a real change, so it clears any lingering margin and stays hot, letting the still-settling notifications through so the refresh converges on the final state. Self-induced churn nets to the same fingerprint and so still opens the margin, which keeps the loop broken. The scanning flag still covers the whole scan (a slow scan outlasts any fixed window), so the only remaining miss is a single-notification change that lands entirely within the short margin after a genuinely no-op scan, which is far rarer than the previous every-scan margin. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ad of blink Every hint refresh cleared the overlay before re-scanning and only redrew once the scan finished. Clearing blanks the window and resets the incremental-draw state, so the redraw is a full one and the overlay sits empty for the whole scan (a couple hundred milliseconds), which reads as a flicker on every refresh. A no-op confirming scan produced a pure off/then/on blink; an Electron app that posts several settling scans produced several blinks in a row. The overlay renderer already diffs the previous hint set against the new one and updates incrementally. Skipping the clear on a refresh lets that path run: an unchanged set renders nothing at all, and a changed set morphs in place with no blank frame. The clear stays on a fresh activation, where it still removes any prior-mode content (e.g. scroll highlights), and a refresh that fails or resolves to no hints still exits through the path that clears. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… not missed Some accessibility changes arrive in two phases. In an Electron app, opening an HTML menu while another is open first dismisses the old menu, then builds the new one. The first phase posts a notification that triggers a scan, and that scan runs while the new menu is still being built. The notification that the new menu finished building lands inside the scan's own suppression window and is dropped, and because nothing else fires afterward, the new menu's items are never scanned and get no hints. Opening the same menu with nothing else open is a single clean event, so it works. When a scan's fingerprint changed (it caught something mid-change), it now proactively schedules one more refresh after the debounce, instead of waiting for another notification that may have been dropped. This re-check keeps firing only while the fingerprint keeps changing and stops once it stabilizes, so a multi-phase change converges on the settled state. A budget caps consecutive re-checks so content that changes every frame (an animation) cannot re-check forever; past the cap, refreshes fall back to being notification-driven. The budget resets whenever a scan settles or a fresh hint session begins. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ols get hints Debug logs of the failing case show the scan does find the new controls (the hint count rises when a second in-page control group opens), so the miss is in drawing, not scanning. The regression traces to no longer clearing the overlay before a refresh: that routed refreshes through the incremental structural draw path, which diffs purely by on-screen position and only does a full replacement when every hint changed. A partial change that keeps most hints but swaps a few (one in-page control group dismissed while another appears, common in web and Electron apps) took the incremental path, where overlapping or colliding positions could leave the newly appeared controls without hints. A fresh activation always does a full redraw, which is why quitting and re-entering hints showed them. Structural changes now fall back to a full redraw. NeruDrawHints replaces the whole hint set in a single repaint with no blank frame, so this is correct and still flicker-free; only the pure-typing case (same hint set, changed input) stays on the incremental match-update path. The now-unused structural incremental helpers are left in place and will be removed in a follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
eac9292 to
9ef6093
Compare
|
Overnight, i had given a bit more thought about the structure of this, and currently I am more inclined in the following way, let me know your thought. config[hints.auto_refresh]
enabled = true # default to false (opt-in basis)
debounce_ms = 150
# user should be able to configure what they want, by default we includes every notifications that we supports
# user can remove them from the list or comment it out
# this has to be validated against `enabled` field, if `auto_refresh` is enabled, `allowed_notifications` has to have at least 1 notification allowed.
# these allowed notifications should also control the observers, and we should only register the observers that are allowed by user config.
allowed_notifications = [
"layout_changed",
"menu_opened",
# and more
]We probably should not make cli
docs
lifecycle
cross platform preparation
A separated PR before this lands could be a scoped additional ax enhancements. |
|
Thanks, I will incorporate the feedback. I've got a set of stacked PRs coming but I cannot open them as stacked because I'm using a downstream repository of yours. I will try to open them as separate PRs and rebase them as they get merged progressively. Unless you wish to add me as a contributor, then I can make a proper stack. Up to you. |
I am fine with separated PRs, will do my best to ensure they are merged as soon as I can. 🤝 |
|
Closing in favor of the chain #1015 #1016 #1017 #1018 I'm not super happy with how it turned out, but I ran out of steam and kind of burned out a bit on working with AI on this, to be honest. In the end, it turns out that the accessibility system does notify on some changes, but not all. As a page progressively loads, you don't get new notifications, which means that we need to implement a kind of exponential back-off re-checking algorithm. That's what I did in #1018. |
|
@y3owk1n by the way, maybe don't go too deep on reviewing these just yet because I haven't had a chance to have a deep look into them (my tooling doesn't let me easily review code before this stage), and I don't wanna waste your time. I need a break from this for a while, but once I'm ready, I'll get back and take a look and clean up a bit. |
Yes, this is always the quirk of working with accessibility API, especially with non native apps, and I can understand how annoying it is. No worries about it, it's a long process, especially on open source projects, take your time when you're ready.
I'll leave some comments and thought scoped to each PR, feel free to read through when you're ready to continue. |
Disclaimer: This code was written by Claude. I steered it toward a solution I thought sensible, and I am opening it as a proof of concept, not a merge candidate.
Status
Do not merge. This is a still-flaky proof of concept for push-based hint auto-refresh, opened to share the approach and gather feedback against #1002. It's a LOT of code. It should probably be done with less code. It is opt-in and off by default, and there is a known miss rate (see Known issues).
Rationale
neru scans the accessibility tree once when hints mode is entered and only re-scans on a few discrete events. If a window's contents change after that scan (a page finishes loading, a banner appears, a menu opens, an Electron view re-renders), the on-screen hints go stale until the user manually refreshes. macOS exposes a push API for exactly this,
AXObserver, which delivers a callback when an app posts an accessibility notification, with no polling.This PR watches the processes a hint scan resolved and re-runs the scan when they actually change, coalesced so bursts collapse into one refresh. The paramount design constraint is resource safety: zero background cost when hints mode is inactive, and no leaked, hanging, or doubled observers or threads.
How it works
CFRunLoopthread servicesAXObservercallbacks. It is created on the first arm and stopped and joined on the last disarm, so an idle neru has no observer thread and no background cost.AXObservercreate, register, and release, plus run-loop source add/remove, all run on that one thread. That serializes them against the observer's own callbacks, so releasing an observer can never race an in-flight callback, and a synchronous AX registration that hangs stalls only the observer thread, never a keystroke or shutdown.Managerowns every observer as an actor: all mutations are commands processed on one goroutine, soReconcile,DisarmAll, app-terminate, andClosecan arrive from different threads without touching the observer map directly.kAXValueChangedis off by default (the noisiest one).Changes
internal/core/infra/platform/darwin/axobserver.{h,m,go}.internal/core/infra/axobserver(manager, per-source masks, platform shim).internal/core/infra/accessibility,internal/core/ports), so coverage tracks the existinginclude_*_hintssettings with no per-source special-casing, and vision-strategy windows never emit a target.[hints.auto_refresh]and[hints.additional_ax_support].Web/Electron accessibility enablement
An Electron or Chromium app exposes no accessibility tree, and posts no notifications, until an assistive tool sets an attribute on it. There are two attributes with very different risk:
AXManualAccessibilitywakes Electron. It is a safe no-op on non-Electron apps and has no window side effect.AXEnhancedUserInterfacewakes Chromium browsers (Chrome, Arc) and Firefox. It is the attribute known to make some apps relayout or move their windows, and it adds real overhead in the target app.The old code only tried
AXManualAccessibilityon a hardcoded Electron whitelist and otherwise went straight to the enhanced attribute. This PR attemptsAXManualAccessibilityon every activated app except neru itself, so Electron apps work without a whitelist, while escalation toAXEnhancedUserInterfacestays gated behind a browser signal so it is never sprayed on native apps (which have no web area and would just get relayouted for nothing). The gate isadditional_ax_support.escalate_enhanced(whitelist | off | all).The whole additional-accessibility path is behind
additional_ax_support.enable, off by default, because waking a browser tree is intrusive by nature (the enhanced attribute's window moves). Native apps need none of this and auto-refresh works on them out of the box.Known issues
Testing
Manager unit tests (self-exclusion, recycled-pid, idempotent double-disarm and double-close, stale-epoch drop, the generation scheme that voids a reconcile queued behind a disarm), all goleak-clean with a fake platform. A native darwin soak test runs many Reconcile/DisarmAll cycles and asserts balanced CFRelease counts and no run-loop thread at idle. Label-reuse tests assert the carried set stays prefix-free. Field-tested by hand on Electron (Claude), the Settings app, and Arc.