test(#277): cross-language header-digest fixture for v2 diagnostic suggestion (#363) - #373
Draft
d-q222 wants to merge 1 commit into
Conversation
…ggestion Fixes #363: proves the Rust (delta::header_digest) and Swift (DisplayListDelta.headerDigest) implementations produce the same dl2-canon-1 header digest for the same input, so a diagnostic's suggestion hashes identically on both sides. Without this, a silent divergence would make the Mac app reject every delta and fall back to full lists. Based on this branch (agent/daniel-parent/mac-v2-diagnostics) rather than main, since it needs the suggestion-hashing change already here. Adds protocol/fixtures/display-list-v2-delta-digest.json: 4 minimal display lists, identical except `diagnostics` (none / one without suggestion / one with suggestion / two mixed), each with its recorded header digest for Wire{diagnostics: true}. A Rust test loads the fixture, rebuilds each DisplayList, and asserts both required_features and header_digest match the recording. A Swift test loads the SAME fixture file in place and asserts DisplayListDelta.headerDigest matches too. No digest disagreement between the two implementations was found. One real bug was found and fixed along the way: the Swift test's fixture-path resolution was off by one directory level (four .deletingLastPathComponent() calls from this file's location land at apps/, not the repo root, so it looked for apps/protocol/fixtures/... instead of protocol/fixtures/...); fixed to five levels. Tests: cargo test -p flashtex-render-pipeline (test header_digests_match_fixture_for_diagnostics_wire): 1 passed, 0 failed. swift test --filter DisplayListDeltaDigestTests from apps/mac: 1 passed, 0 failed -- confirming the cross-language agreement this PR exists to prove. Both re-run independently by the reviewer; the Swift half specifically could not be executed inside the Muse sandbox (SwiftPM's own internal sandboxing can't nest inside the outer one), so it was verified by the reviewer outside that sandbox, including finding and routing the path bug back to the same lane for a proper fix rather than patching it directly. Implementation-Agent: muse-spark-1.3-contributor (Muse Code, lane delta-digest-cross-language, slices 1-2) Commit-Executor: daniel-muse-lead (Claude Sonnet) Reviewed-by: daniel-muse-lead (Claude Sonnet) Muse-Lane-Head: 66ac030b8b1a81d0edd8595e13704980a508d7a9 Co-authored-by: d-q222 <279808976+d-q222@users.noreply.github.com>
This was referenced Sep 14, 2026
Contributor
Author
|
daniel-parent review: approve on substance (a comment, since we share the account). Verified independently on this branch:
So Rust and Swift agree on the |
d-q222
added a commit
that referenced
this pull request
Sep 14, 2026
Follow-up to #374 (review feedback): the merged fix's tests injected an immediate-perform scheduler ({ _, item in item.perform() }), so testOnlyTheLastScheduledUpdateWins only exercised the generation-counter guard -- the first scheduleUpdate's item always ran to completion before the second call happened, so debounce?.cancel() actually superseding a still-pending item (the real production coalescing path) was never tested. Adds testCancelledScheduleNeverPublishesItsResult using a capturing scheduler ({ _, item in captured.append(item) }) instead: neither item runs until explicitly told to, so the second scheduleUpdate call must cancel the still-pending first item. Asserts both calls reach the seam (2 items captured), the first is marked cancelled, and -- even performing both items manually, simulating what would happen if a queue naively ran a "cancelled" item, which real DispatchQueue.asyncAfter would not do -- only the second update's word count is published, confirming the generation guard as a second independent line of defense. testOnlyTheLastScheduledUpdateWins, testScheduleUpdateEventuallyPublishesTotals, and WordCountModel itself are unchanged; this is a test-only addition using the scheduleDebounce seam #374 already added. Tests: swift test --filter 'WordCountModelTests|DocumentStatisticsTests', 3 consecutive runs, all green (26 tests executed, 2 skipped, 0 failures each run). Run outside the Muse sandbox by the reviewer, as for #373/#374. Implementation-Agent: muse-spark-1.3-contributor (Muse Code, lane wordcount-cancel-coverage, slice 1) Commit-Executor: daniel-muse-lead (Claude Sonnet) Reviewed-by: daniel-muse-lead (Claude Sonnet) Muse-Lane-Head: a0d844394f28fef47ecf42d4392fcc3d4bbaf104 Co-authored-by: d-q222 <279808976+d-q222@users.noreply.github.com>
d-q222
added a commit
that referenced
this pull request
Sep 14, 2026
Follow-up to #374 (review feedback): the merged fix's tests injected an immediate-perform scheduler ({ _, item in item.perform() }), so testOnlyTheLastScheduledUpdateWins only exercised the generation-counter guard -- the first scheduleUpdate's item always ran to completion before the second call happened, so debounce?.cancel() actually superseding a still-pending item (the real production coalescing path) was never tested. Adds testCancelledScheduleNeverPublishesItsResult using a capturing scheduler ({ _, item in captured.append(item) }) instead: neither item runs until explicitly told to, so the second scheduleUpdate call must cancel the still-pending first item. Asserts both calls reach the seam (2 items captured), the first is marked cancelled, and -- even performing both items manually, simulating what would happen if a queue naively ran a "cancelled" item, which real DispatchQueue.asyncAfter would not do -- only the second update's word count is published, confirming the generation guard as a second independent line of defense. testOnlyTheLastScheduledUpdateWins, testScheduleUpdateEventuallyPublishesTotals, and WordCountModel itself are unchanged; this is a test-only addition using the scheduleDebounce seam #374 already added. Tests: swift test --filter 'WordCountModelTests|DocumentStatisticsTests', 3 consecutive runs, all green (26 tests executed, 2 skipped, 0 failures each run). Run outside the Muse sandbox by the reviewer, as for #373/#374. Implementation-Agent: muse-spark-1.3-contributor (Muse Code, lane wordcount-cancel-coverage, slice 1) Commit-Executor: daniel-muse-lead (Claude Sonnet) Reviewed-by: daniel-muse-lead (Claude Sonnet) Muse-Lane-Head: a0d844394f28fef47ecf42d4392fcc3d4bbaf104 Co-authored-by: d-q222 <279808976+d-q222@users.noreply.github.com>
GoKubar
pushed a commit
that referenced
this pull request
Sep 15, 2026
…406) * mac: recomputeExecutor seam to fix the WordCount full-suite load flake PR #374 fixed the debounce timer's flakiness but left recompute's background-dispatch-and-MainActor-publish hop on a real queue, which a maintainer reproduced failing 3/1133 under full-suite load even though it passed reliably in isolation. Adds recomputeExecutor (mirrors the existing scheduleDebounce seam), defaulting to the real background queue in production; tests inject a synchronous executor plus a deterministic MainActor-drain (await a Task enqueued after the publish task, relying on serial-executor FIFO order, no wall-clock poll) instead of raising settles()'s timeout. Verified, measured: cherry-picked onto current main and ran the full Mac suite (1136 tests) three consecutive times outside the sandbox -- 0 failures each run (previously 3 failures observed in a comparable 1133-test run per the original report). Implementation-Agent: muse-spark-1.3-contributor (Muse Code, lane wordcount-recompute-seam, slice 1) Commit-Executor: daniel-muse-lead (Claude Sonnet) Reviewed-by: daniel-muse-lead (Claude Sonnet) Muse-Lane-Head: c5c29708 Co-authored-by: muse-spark-1.3-contributor <muse-contributor@flashtex.invalid> * mac: wait on the actual publication, not Task enqueue order Fixes a real correctness gap in the previous commit, found by review: published()'s drain trick assumed an unstructured Task { @mainactor } created after recompute's own publish Task would run after it, purely because it was enqueued later. That ordering is not a documented Swift concurrency guarantee. Replaces it with an explicit onRecomputeSettled hook: recompute's Task calls it (via defer, so it fires whether the generation guard published or superseded) right after its publish decision is made. Production leaves it nil, at no cost. Tests arm a checked continuation that resumes only once the expected number of settle events has occurred (some tests trigger two updates and expect exactly one, the second, to publish -- both still reach a settle decision). Verified: 3 consecutive full Mac suite runs (1143 tests each) after this fix, all with 0 failures in WordCountModelTests specifically (confirmed both embedded in the full run and in isolation via `swift test --filter WordCountModelTests`). Each full run also shows 2 failures (0 unexpected) elsewhere in the suite, unrelated to this change and stable/identical across all 3 runs -- pre-existing, not introduced here. Implementation-Agent: daniel-muse-lead (Claude Sonnet, fixing muse-spark-1.3-contributor's earlier commit per daniel-parent's review) Commit-Executor: daniel-muse-lead (Claude Sonnet) Reviewed-by: daniel-muse-lead (Claude Sonnet) * WordCountModel: test that a cancelled debounce item never publishes Follow-up to #374 (review feedback): the merged fix's tests injected an immediate-perform scheduler ({ _, item in item.perform() }), so testOnlyTheLastScheduledUpdateWins only exercised the generation-counter guard -- the first scheduleUpdate's item always ran to completion before the second call happened, so debounce?.cancel() actually superseding a still-pending item (the real production coalescing path) was never tested. Adds testCancelledScheduleNeverPublishesItsResult using a capturing scheduler ({ _, item in captured.append(item) }) instead: neither item runs until explicitly told to, so the second scheduleUpdate call must cancel the still-pending first item. Asserts both calls reach the seam (2 items captured), the first is marked cancelled, and -- even performing both items manually, simulating what would happen if a queue naively ran a "cancelled" item, which real DispatchQueue.asyncAfter would not do -- only the second update's word count is published, confirming the generation guard as a second independent line of defense. testOnlyTheLastScheduledUpdateWins, testScheduleUpdateEventuallyPublishesTotals, and WordCountModel itself are unchanged; this is a test-only addition using the scheduleDebounce seam #374 already added. Tests: swift test --filter 'WordCountModelTests|DocumentStatisticsTests', 3 consecutive runs, all green (26 tests executed, 2 skipped, 0 failures each run). Run outside the Muse sandbox by the reviewer, as for #373/#374. Implementation-Agent: muse-spark-1.3-contributor (Muse Code, lane wordcount-cancel-coverage, slice 1) Commit-Executor: daniel-muse-lead (Claude Sonnet) Reviewed-by: daniel-muse-lead (Claude Sonnet) Muse-Lane-Head: a0d844394f28fef47ecf42d4392fcc3d4bbaf104 Co-authored-by: d-q222 <279808976+d-q222@users.noreply.github.com> * mac: fix two real bugs in the cancelled-schedule test (REQUEST_CHANGES) daniel-parent's review found two real problems in #406's testCancelledScheduleNeverPublishesItsResult: 1. DispatchWorkItem.perform() on a cancelled item is a no-op -- it never reaches recompute at all (confirmed by instrumentation: a print at recompute's entry never fires for the cancelled item). So calling captured[0].perform() only ever exercised DispatchWorkItem's own cancellation, never this codebase's `generation` guard as an independent second line of defense, no matter how the test was worded. Fixed by widening `recompute` from private to internal (an @testable-only surface) and calling it directly with a stale generation, bypassing DispatchWorkItem entirely. Mutation-tested: temporarily removing the `gen == self.generation` check now fails this test (confirmed, then reverted); it did not fail before this fix. 2. FLASHTEX_WORDCOUNT_DEBOUNCE_MS=0 would have made captured[0] crash (empty array). Guarded with XCTSkipIf. Also rebased onto #418 (the recomputeExecutor seam) to use its makeModel()/published() helpers instead of the wall-clock settles() this branch still referenced pre-rebase. Implementation-Agent: muse-spark-1.3-contributor (Muse Code, lane wordcount-cancel-coverage, slice 1) + daniel-muse-lead (Claude Sonnet, review fix-up) Commit-Executor: daniel-muse-lead (Claude Sonnet) Reviewed-by: daniel-muse-lead (Claude Sonnet) --------- Co-authored-by: muse-spark-1.3-contributor <muse-contributor@flashtex.invalid>
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.
daniel-muse-lead — Muse Contributor lane
delta-digest-cross-language, slices 1-2. Delegated by daniel-parent (#363). Based onagent/daniel-parent/mac-v2-diagnostics, notmain, since it needs thesuggestion-hashing change already there.Proves the Rust (
delta::header_digest) and Swift (DisplayListDelta.headerDigest) implementations produce the samedl2-canon-1header digest for the same input. No divergence found.protocol/fixtures/display-list-v2-delta-digest.json: 4 minimal display lists, identical exceptdiagnostics(none / one withoutsuggestion/ one with / two mixed), each with its recorded digest forWire{diagnostics: true}.DisplayList, asserts bothrequired_featuresandheader_digestmatch.DisplayListDelta.headerDigestmatches too — this is what actually proves cross-language agreement.One real bug found and fixed along the way: the Swift test's fixture-path resolution was off by one directory level (
apps/protocol/fixtures/...instead ofprotocol/fixtures/...) — four.deletingLastPathComponent()calls from the test file's location land atapps/, not repo root. The Muse sandbox can't runswift test(SwiftPM's own internal sandboxing can't nest inside the outer one), so this went undetected in-sandbox; I found it by running the test myself outside that sandbox, then routed the one-line fix back to the same lane as slice 2 rather than patching it directly, to keep provenance truthful.Gate:
cargo test -p flashtex-render-pipeline: 1 passed, 0 failed.swift test --filter DisplayListDeltaDigestTestsfromapps/mac: 1 passed, 0 failed — re-run by me, outside the Muse sandbox, confirming the actual cross-language agreement this PR exists to prove.Implementation-Agent: muse-spark-1.3-contributor (Muse Code, lane delta-digest-cross-language, slices 1-2)
Commit-Executor: daniel-muse-lead (Claude Sonnet)
Reviewed-by: daniel-muse-lead (Claude Sonnet)
🤖 Generated with Claude Code