test: pin the frame-loop invariants behind the #35 fixes (tests now run in CI) - #37
Merged
Merged
Conversation
The #35 diagnosis came from ad-hoc instrumentation; the parts of it that are deterministic are now CI-tested so the same regressions cannot silently return: - TextMeasurer (extracted from DasherCanvas): the dasher.h return-code contract (0 = success, non-zero = fall back to the engine estimate) — v0.1.17 inverted this and the engine discarded every measurement while re-calling ~21x per frame; plus cache semantics (repeat hits, keyed by text+font+size, Invalidate on font change). Measurement backend is injectable so the tests need no font manager - EngineTimeline (extracted from StepFrame): the engine-time clamp — pause gaps and scheduler hiccups must never reach the engine as huge time deltas (it consumes them as zoom amount); degenerate/negative deltas step the minimum - EngineCApiTests (integration, real dasher.dll + DasherCore/Data): the permitted-values probe returns the full count before fetch (DasherCore #58, which the settings dropdowns rely on), and the text-size callback stops firing once the engine cache is warm (the regression shape of #33/#36). These skip when the engine artifacts are absent locally; CI builds the DLL first - Build Installer workflow now runs dotnet test after building dasher.dll, before publish Frame pacing itself (RAF cadence, jitter) stays manual verification: CI runners have no real compositor and timing assertions would flake. Signed-off-by: will wade <willwade@gmail.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ping TryCreateEngine now distinguishes missing artifacts (legit local skip) from dasher_create returning a null handle with artifacts present — the latter must fail, or an ABI/init regression would pass undetected (caught in review of #37). Signed-off-by: will wade <willwade@gmail.com>
| return EngineAvailability.Ready; | ||
| } | ||
| catch (DllNotFoundException) { return EngineAvailability.ArtifactsMissing; } | ||
| catch (BadImageFormatException) { return EngineAvailability.ArtifactsMissing; } |
There was a problem hiding this comment.
When CI supplies a corrupt, incompatible, or wrong-architecture dasher.dll, BadImageFormatException is classified as ArtifactsMissing, causing both native integration tests to return without assertions and pass despite the unusable native build.
Suggested change
| catch (BadImageFormatException) { return EngineAvailability.ArtifactsMissing; } |
Writing the engine integration tests surfaced a real DasherCore bug: dasher_create threw regex_error whenever the settings file did not exist yet and the user-dir path contained a backslash escape MSVC's std::regex rejects (digit-leading components parse as backreferences, 'c'-leading as invalid control escapes) - ~70% of fresh GUID temp dirs. The Windows tests were failing non-deterministically on exactly that dice roll. Fixed upstream in DasherCore #59 (ScanFiles never compiles an absolute path as a regex); submodule bumped to v0.2.6. Also makes the CI engine tests impossible to skip silently: the Run tests step sets DASHER_TESTS_REQUIRE_ENGINE=1 (with the freshly built DLL on PATH), so missing artifacts or an init failure fails the build - previously the integration tests passed vacuously on CI (105 ms total) because dasher.dll/Data were not resolvable in the test host, so CI green said nothing about the ABI. Local runs without artifacts still skip. Signed-off-by: will wade <willwade@gmail.com>
Merged
3 tasks
willwade
added a commit
that referenced
this pull request
Sep 8, 2026
…oard bridge (RFC 0015) (#52) ## Summary Implements the frontend half of the RFC 0015 context-awareness amendment for Windows (governance PR #37; research in #50; engine CAPI in DasherCore PR #83 — submodule pinned to that branch pending merge/tag). Dasher 5's "direct mode knows the context" was really a session shadow buffer — it never read the target field. This PR restores that parity and exceeds it: predictions now continue from **text already in the target field**, which v5 never could. ### Context seeding (RFC 0015 tiers 1–3) - **`TargetContextReader`** (`Services/TargetContextReader.cs`): reads the focused control's text + caret via **UI Automation `TextPattern`** (modern apps: WPF, UWP, browsers, Office, Electron) with a **`WM_GETTEXT`/`EM_GETSEL` fallback** for classic EDIT/RichEdit controls (Notepad, dialogs). All reads run on a background thread with a **300 ms hard budget** — an unresponsive provider can never stall a mode switch; failures return `null`. - **Seeding triggers**: - *Keyboard-mode entry* — reads the field you were typing in, so predictions continue mid-sentence - *Target-window change* — switching fields **re-reads and re-seeds** the new field (better than v5, which reset to empty); read failure degrades to empty seed (v5 parity, never a dead mode) - **Caret conversion**: UIA reports UTF-16 code units; the engine wants UTF-8 bytes — converted with the new `dasher_byte_offset_from_utf16` CAPI (the shared, tested converter from DasherCore #83), so CJK/accented text anchors correctly - 150 ms debounce for focus churn; `KbLog` traces each seed ### Clipboard bridge (mini-bar) Four new buttons on the keyboard-mode mini-bar: - **Copy** — engine buffer → system clipboard - **Cut / Paste / Select-All** — `Ctrl+X/V/A` chords injected into the target via `SendInput`; they act on the *target's* selection, which only the target can do — the same reason v5 implemented them frontend-side rather than in control mode ## Verification - Build green, 77/77 tests - Needs a manual pass: type into Notepad (fallback path), a browser textarea (UIA path), switch between two apps (re-seed), non-ASCII text (conversion), the four mini-bar buttons ## Depends on - DasherCore PR #83 (`dasher_seed_buffer`, `dasher_set_offset`, unit converters) — submodule pinned to `feat/context-capi`; **re-pin to the tagged release before merge** ## Type of change - [x] New feature (cross-platform parity + capability v5 lacked) ## Definition of Done - [x] Build + tests green - [x] Commits signed off (DCO) <!-- greptile_comment --> <h3>Greptile Summary</h3> The PR adds target-field context seeding and clipboard controls for direct keyboard mode, with centralized target-window tracking and ownership checks. - Reads focused-field text and caret positions through UI Automation with a Win32 edit-control fallback. - Re-seeds predictions on foreground and same-window focus changes. - Adds Copy, Cut, Paste, and Select-All controls to the keyboard mini-bar. - Adds target-window identity and UI regression tests. <h3>Confidence Score: 5/5</h3> The PR appears safe to merge. No blocking failure remains. <details><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | src/Dasher.Windows/Services/TargetContextReader.cs | Adds bounded target-context reads, ownership validation, and layered caret discovery; the latest rcCaret fix resolves the previously reported cross-thread caret-coordinate defect. | | src/Dasher.Windows/Services/KeyboardTargetTracker.cs | Centralizes rooted target tracking, focus and foreground hooks, stale-event handling, and foreground restoration. | | src/Dasher.Windows/Services/TargetWindowIdentity.cs | Defines and tests a shared GA_ROOT-based target ownership predicate. | | src/Dasher.Windows/Views/MainWindow.axaml.cs | Integrates debounced context seeding, generation invalidation, target tracking, and clipboard bridge actions. | | src/Dasher.Windows/Views/MainWindow.axaml | Adds keyboard mini-bar controls for copying engine text and sending target-side Cut, Paste, and Select-All commands. | </details> <details open><summary><h3>Sequence Diagram</h3></summary> ```mermaid sequenceDiagram participant W as WinEvent hooks participant T as KeyboardTargetTracker participant M as MainWindow participant R as TargetContextReader participant E as DasherCore W->>T: Foreground or focus event T->>T: Root and validate target HWND T->>M: TargetChanged M->>M: Debounce and generation check M->>T: Verify target foreground M->>R: ReadAsync(target HWND) R->>R: UIA TextPattern or Win32 fallback R-->>M: Text and UTF-16 caret M->>E: Convert caret to UTF-8 offset M->>E: Seed buffer ``` </details> <sub>Reviews (19): Last reviewed commit: ["fix(keyboard): caret rect from GUITHREAD..."](6726fc3) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=61122413)</sub> <!-- /greptile_comment --> --------- Signed-off-by: will wade <willwade@gmail.com> Co-authored-by: will wade <will wade@gmail.com>
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
1 participant
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
Follow-up to #36: the choppiness diagnosis came from ad-hoc instrumentation. The deterministic parts of what that instrumentation revealed are now pinned as tests so the same regressions can't silently return; the genuinely timing-based parts (RAF cadence, jitter) stay manual — CI runners have no real compositor and timing assertions would flake.
TextMeasurerTests.Measure_success_returns_zero_per_dasher_h_contractInvalidateon font change)TextMeasurerTests(cache suite)EngineTimelineTests.StepFor_*EngineCApiTests.Text_size_callback_hits_engine_cache_in_steady_state— integration vs the realdasher.dll, catches an inverted interpretation on either side of the ABIEngineCApiTests.Permitted_values_probe_returns_full_count_before_fetchImplementation notes:
TextMeasurerandEngineTimelineextracted fromDasherCanvas(behaviour unchanged; the measurement backend is injectable so unit tests need no font manager — Avalonia 12's headless API churn made that the robust route).NativeBridgedirectly with the realdasher.dll+DasherCore/Data; they pass vacuously if the artifacts are absent (local dev without the DLL) and bite in CI.dotnet testafter buildingdasher.dll, before publish — so every PR and release build runs all 59 tests including the engine integrations.Type of change
Cross-platform impact
Definition of Done
Greptile Summary
The PR extracts frame-timeline and text-measurement behavior into testable components and adds native-engine integration coverage to CI.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Reviews (3): Last reviewed commit: "test: require engine in CI; bump DasherC..." | Re-trigger Greptile