From 72e0113acdb137fea18ae650e2ff4607fe413e8f Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Sun, 5 Jul 2026 22:47:19 +0200 Subject: [PATCH] fix a bunch of fuzz tests --- src/ytype.js | 79 +++++++++++++++++++--------- tests/attribution.tests.js | 105 +++++++++++++++++++++---------------- 2 files changed, 116 insertions(+), 68 deletions(-) diff --git a/src/ytype.js b/src/ytype.js index 2240ff1a..1bd6e6cb 100644 --- a/src/ytype.js +++ b/src/ytype.js @@ -1059,6 +1059,15 @@ export class YType extends ObservableV2 { * @type {delta.Formats} */ const previousFormats = {} // The value before changes + /** + * Per-key provenance of `currentFormats`: `true` iff the marker currently governing the key + * opened a *pending attributed* range (the attributor-push branch below). Consulted only at + * deleted markers in change renders (`isDeletedFormatClear`) to distinguish a re-exposed + * committed enclosing value (must clear the stale attribution from the cache) from a + * re-exposed pending suggested value (must preserve it — the re-bolding case). + * @type {{ [key: string]: boolean }} + */ + const currentFormatsAttributed = {} /** * Attribution argument for a change-render retain whose content lost its *own* attribution. * `null` clears everything — correct in an attribution-free context — but inside an ambient @@ -1208,31 +1217,33 @@ export class YType extends ObservableV2 { if (renderedFormatKeys === null) renderedFormatKeys = new Set() renderedFormatKeys.add(key) } + // `previousFormats` tracks the value governing the current walk position in the + // *consuming cache state*, so the retain diff below stays a plain cache → new-state + // comparison. Contributors: markers deleted by this change in event renders (their + // value governed the cache until now; heal-rendered deletes are PRE-deleted markers + // whose removal the cache already rendered — they must not contribute), alive + // markers re-rendered by heal renders (`retainInserts` — their value was in the + // cache), and retained alive markers (the branch below). A gated update here is not + // enough: a marker deleted by the change whose value coincides with the walk's + // current value would silently drop out of the cache-reference tracking, and a later + // marker would be misread as a restore-to-previous, swallowing a needed format diff. if (c.deleted) { - // content was deleted, but is possibly attributed - if (!equalFormats(value, currFormatVal)) { // do nothing if nothing changed - if (equalFormats(currFormatVal, previousFormats[key] ?? null) && changedFormats[key] !== undefined) { - delete changedFormats[key] - } else { - changedFormats[key] = currFormatVal - } - // current formats doesn't change - previousFormats[key] = value - } + if (itemsToRender !== null && !retainInserts) previousFormats[key] = value } else { // !c.deleted - // content was inserted, and is possibly attributed - if (equalFormats(value, currFormatVal)) { - // item.delete(transaction) - } else if (equalFormats(value, previousFormats[key] ?? null)) { - delete changedFormats[key] - } else { - changedFormats[key] = value - } + if (retainInserts) previousFormats[key] = value if (value == null) { delete currentFormats[key] } else { currentFormats[key] = value } + currentFormatsAttributed[key] = false + } + // the retain diff for following spans is exactly cache (previousFormats) → new + // state (currentFormats), recomputed per key at every rendered marker + if (equalFormats(currentFormats[key] ?? null, previousFormats[key] ?? null)) { + delete changedFormats[key] + } else { + changedFormats[key] = currentFormats[key] ?? null } } else if (retainContent && !c.deleted) { // fresh reference to currentFormats only @@ -1249,6 +1260,7 @@ export class YType extends ObservableV2 { } else { currentFormats[key] = value } + currentFormatsAttributed[key] = false delete changedFormats[key] previousFormats[key] = value } @@ -1259,11 +1271,19 @@ export class YType extends ObservableV2 { // `{ format: { [key]: [] } }` the marker's insertion wrote to the cache. Emit an explicit // `null` leaf for the key (a context-wide `useAttribution(null)` cannot carry a per-key // clear). Conditions: only an attributing render (`renderer !== null`; without a - // renderer there are no attributions to clear), and only when the deletion actually *removes* - // the format — i.e. it reverts to no value (`currFormatVal == null`). If it reverts to a - // still-present surrounding value (e.g. deleting a `bold:null` marker re-exposes an - // enclosing attributed `bold:true`, as when re-bolding), the attribution is preserved. - const isDeletedFormatClear = attribution == null && renderer !== null && renderDelete && c.deleted && itemsToRender != null && currFormatVal == null && !equalFormats(value, currFormatVal) + // renderer there are no attributions to clear), and only when the deletion reverts to + // a value that is not itself governed by a pending suggestion: either no value at all + // (`currFormatVal == null`) or a *committed* enclosing value (provenance tracked in + // `currentFormatsAttributed` — e.g. an accept/reject deleting a marker re-exposes the + // enclosing committed `bold:true`, and the span's stale suggested-format attribution + // must be cleared). If it reverts to a still-pending *attributed* value (deleting a + // `bold:null` marker re-exposes an enclosing attributed `bold:true`, as when + // re-bolding), the attribution is preserved. Suppressed entirely while an attributed + // same-key range is open (`previousUnattributedFormats` has the key): there the fresh + // render skips this marker — its governance belongs to the still-open attributed + // range, whose ambient context re-stamps the correct attribution on the following + // retains — mirroring the identical guard on `isAcceptedFormatClear`. + const isDeletedFormatClear = attribution == null && renderer !== null && renderDelete && c.deleted && itemsToRender != null && (currFormatVal == null || currentFormatsAttributed[key] !== true) && !equalFormats(value, currFormatVal) && !object.hasProperty(previousUnattributedFormats, key) // The alive-marker analogue of `isDeletedFormatClear`: a format marker whose // *attribution* was removed while the marker survives — its suggestion was accepted // (the marker's id arrives via the renderer's `'change'` event). The original change @@ -1336,6 +1356,11 @@ export class YType extends ObservableV2 { } delete previousUnattributedFormats[key] } else { + // an attributed marker opens pending governance for the key — a deleted attributed + // marker never writes `currentFormats`, hence the alive gate; an attributed marker + // that value-equals the previous unattributed value takes the reset branch above + // and correctly yields committed governance + if (!c.deleted) currentFormatsAttributed[key] = true const by = changedAttributedFormats[key] = (changedAttributedFormats[key]?.slice() ?? []) by.push(...((c.deleted ? attribution.delete : attribution.insert) ?? [])) const attributedAt = (c.deleted ? attribution.deleteAt : attribution.insertAt) @@ -1343,7 +1368,13 @@ export class YType extends ObservableV2 { } if (object.isEmpty(changedAttributedFormats)) { d.useAttribution(null) - } else if (attribution != null || isDeletedFormatClear || isAcceptedFormatClear || isRangeEndClear) { + } else if (attribution != null || isDeletedFormatClear || isAcceptedFormatClear || isRangeEndClear || (itemsToRender != null && object.every(changedAttributedFormats, v => v == null))) { + // the last disjunct: a change-render copy whose remaining leaves are ALL per-key + // null clears is the analogue of the fresh render's empty copy (fresh contexts + // never contain null leaves — they are produced only by the change-render-only + // predicates above) — install it, so an unattributed close marker's key deletion + // closes the governance while the pending cache clears still propagate; discarding + // it would re-merge the stale key into the cache from the surviving ambient const attributedAt = (c.deleted ? attribution?.deleteAt : attribution?.insertAt) if (attributedAt != null) formattingAttribution.formatAt = attributedAt d.useAttribution(formattingAttribution) diff --git a/tests/attribution.tests.js b/tests/attribution.tests.js index 3e613ee3..d47c87e7 100644 --- a/tests/attribution.tests.js +++ b/tests/attribution.tests.js @@ -1530,13 +1530,14 @@ export const testRdtPartialAcceptKeepsPendingChildSuggestions = () => { } /** - * Currently-failing repro (minimized from y-prosemirror's `.dbg-fuzz.mjs` seed 54321, op #8; - * 14/25 fuzz seeds hit this class): a *base-doc* format arrives over an overlapping same-key - * *suggested* format, then a plain base-doc insert lands inside the formatted range (past the - * suggested span, with formatted content still following) — the change render emitted for the - * insert fails to close the attributed-format range, so the maintained `.delta` cache keeps a - * stale `{format: {strong: []}}` on the trailing content while a fresh render shows it - * committed: + * Regression pin — fixed by `isRangeEndClear` in the ytype format walk (a rendered unattributed + * marker closing an attributed same-key range must emit the per-key attribution clear). + * Minimized from y-prosemirror's `.dbg-fuzz.mjs` seed 54321, op #8; 14/25 fuzz seeds hit this + * class: a *base-doc* format arrives over an overlapping same-key *suggested* format, then a + * plain base-doc insert lands inside the formatted range (past the suggested span, with + * formatted content still following) — the change render emitted for the insert failed to close + * the attributed-format range, so the maintained `.delta` cache kept a stale + * `{format: {strong: []}}` on the trailing content while a fresh render showed it committed: * * cached: a(strong) | bx({format:{strong:[]}}) | c(strong, {format:{strong:[]}} ← STALE) * fresh : a(strong) | bx({format:{strong:[]}}) | c(strong) @@ -1577,22 +1578,21 @@ export const testRdtBaseInsertIntoOverlappingFormatRangeCacheDrift = () => { } /** - * Currently-failing sibling of the class above, found by grid-enumerating the repro skeleton - * (192 same-class combos in the suggested-insert pass, 496 more with formatted base inserts; - * pre-existing — before `isRangeEndClear` it drifted one op earlier, at the base format op): - * with TWO overlapping suggested format keys the ambient attributed format context never - * empties, so when a base-doc format arrives on one of the keys, no reset — neither the fresh - * render's `useAttribution(null)` path nor `isRangeEndClear`, which deliberately mirrors it — - * closes that key's governance. The stale `{format: {strong: []}}` then surfaces in the cache - * on the next change render that walks the span: a suggested insert *inside* the em range - * (an insert past it renders clean): + * Regression pin — fixed by installing the all-null-leaves context copy in the attribution + * install gate. Sibling of the class above, found by grid-enumerating the repro skeleton + * (192 same-class combos in the suggested-insert pass, 496 more with formatted base inserts): + * with TWO overlapping suggested format keys, the change-render copy of the ambient attributed + * format context carries per-key null-clear leaves (which fresh contexts never hold), so the + * emptiness test at the install gate failed where the fresh walk reaches its empty-copy + * `useAttribution(null)` closure — an unattributed close marker's key deletion was discarded. + * The stale `{format: {strong: []}}` then surfaced in the cache on the next change render that + * walks the span: a suggested insert *inside* the em range (an insert past it renders clean): * * cached: a(em+strong, both attributed) | x({insert:[]}) | b(…) | cd({format:{strong:[]}} ← STALE) * fresh : a(em+strong, both attributed) | x({insert:[]}) | b(…) | cd * * The marker integration order is load-bearing: it reproduces with base.clientID > sugg.clientID - * and renders clean with the order flipped. Closing it needs real multi-key closure of attributed - * format contexts (per-key, not only-key) in both the fresh and the change render walks. + * and renders clean with the order flipped. */ export const testRdtSuggestedInsertUnderTwoKeySuggestedFormatsCacheDrift = () => { const base = new Y.Doc({ gc: false }) @@ -1624,8 +1624,9 @@ export const testRdtSuggestedInsertUnderTwoKeySuggestedFormatsCacheDrift = () => } /** - * Currently-failing formatted-base-insert sibling of the two-key class above (grid class B, - * 496 combos in the formatted-base-insert fuzzing pass; pre-existing): under a two-key + * Regression pin — fixed by the same install-gate change as the two-key class above (the + * all-null-leaves context copy must be installed). Formatted-base-insert sibling (grid class B, + * 496 combos in the formatted-base-insert fuzzing pass): under a two-key * suggested format context (em over 'ab', strong over 'a'), a base-doc em arrives on 'a' * only — strictly undercovering the suggested em range — so the em governance over the * trailing 'b' stays attributed-stale in the cache. A base insert *formatted with the other @@ -1671,11 +1672,14 @@ export const testRdtFormattedBaseInsertUnderTwoKeySuggestedFormatsCacheDrift = ( } /** - * Currently-failing FORMAT-VALUE divergence (pre-existing; minimized from fuzz-core seeds 711 - * and 4935, which both collapse to this one 4-op skeleton — the two seeds are mirror - * directions of it, `em:{}` leaking format IN vs `em:null` leaking format OUT): + * Regression pin — fixed by the `previousFormats` cache-reference rework in `# Update Formats` + * (a marker deleted by the change whose value equals the walk's current value dropped out of + * the tracking, so a later marker was misread as a restore-to-previous and the needed format + * diff was swallowed). FORMAT-VALUE divergence, minimized from fuzz-core seeds 711 and 4935, + * which both collapse to this one 4-op skeleton — the two seeds are mirror directions of it, + * `em:{}` leaking format IN vs `em:null` leaking format OUT: * when a base-doc format op's range END lands strictly *inside* a same-key suggested-format - * span, the change render stamps the base op's format one char PAST its range end. The cache + * span, the change render stamped the base op's format one char PAST its range end. The cache * then disagrees with a fresh deep render about the EFFECTIVE FORMAT of that boundary char — * not merely its attribution (no attribution differs; none is even present): * @@ -1720,10 +1724,13 @@ export const testRdtBaseFormatEndingInsideSuggestedFormatRangeFormatValueCacheDr } /** - * Currently-failing MISSING-attribution class (inverse direction of the stale classes above — - * there the cache keeps an attribution the fresh render has dropped; here the cache DROPS one - * the fresh render keeps). Minimized from fuzz-core seed 2 (drift at opIndex 24, 25 ops → 3): - * a *suggested format removal* (key → null) of a base-doc format is applied to the maintained + * Regression pin — fixed by the open-attributed-range guard on `isDeletedFormatClear` + * (`previousUnattributedFormats` has the key → the fresh render skips the deleted marker, so + * the change render must not emit a clear there; mirrors `isAcceptedFormatClear`'s guard). + * MISSING-attribution class (inverse direction of the stale classes above — there the cache + * keeps an attribution the fresh render has dropped; here the cache DROPPED one the fresh + * render keeps). Minimized from fuzz-core seed 2 (drift at opIndex 24, 25 ops → 3): + * a *suggested format removal* (key → null) of a base-doc format was applied to the maintained * cache without the attributed-removal marker that the fresh render produces: * * cached: a(em) | b ← removal applied, attribution LOST @@ -1764,13 +1771,15 @@ export const testRdtSuggestedRemovalOfOverriddenBaseFormatCacheDrift = () => { } /** - * Adjudicated core of lifecycle-fuzz class P5 ("post-clearCache re-drift", seeds 657/1131/2401): - * clearCache is NOT load-bearing — all three seeds drift identically with the clearCache op - * deleted, and a FRESH doc/renderer pair built via encodeStateAsUpdate/applyUpdate to the - * pre-clearCache state drifts identically on the same final op. The class collapses into the - * core no-lifecycle overlapping same-key format drift family (missing-attribution direction): - * a suggested unformat of a base-doc format, staged left-to-right in two steps, loses the - * second step's unformat attribution in the maintained cache: + * Regression pin — fixed by the same open-attributed-range guard on `isDeletedFormatClear` as + * the class above. Adjudicated core of lifecycle-fuzz class P5 ("post-clearCache re-drift", + * seeds 657/1131/2401): clearCache is NOT load-bearing — all three seeds drift identically with + * the clearCache op deleted, and a FRESH doc/renderer pair built via + * encodeStateAsUpdate/applyUpdate to the pre-clearCache state drifts identically on the same + * final op. The class collapses into the core no-lifecycle overlapping same-key format drift + * family (missing-attribution direction): a suggested unformat of a base-doc format, staged + * left-to-right in two steps, lost the second step's unformat attribution in the maintained + * cache: * * cached: a({format:{em:[]}}) | b(no attribution ← MISSING) * fresh : ab({format:{em:[]}}) @@ -1813,9 +1822,13 @@ export const testRdtStagedSuggestedUnformatOfBaseFormatCacheDrift = () => { } /** - * Currently-failing repro (lifecycle-fuzz class P2, 23/3000 seeds; minimized from seed 259's - * 9 ops to 3): ACCEPT-triggered cache drift — every step before the accept is consistent, and - * `acceptAllChanges()` itself corrupts the maintained `.delta`. Shape: a committed `strong` run + * Regression pin — fixed by provenance tracking (`currentFormatsAttributed`) widening + * `isDeletedFormatClear` to committed enclosing values: the accept deletes the committed run's + * close marker, whose removal re-exposes the committed `strong={}`, and the old + * `currFormatVal == null` guard blocked the needed clear. (Lifecycle-fuzz class P2, 23/3000 + * seeds; minimized from seed 259's 9 ops to 3): ACCEPT-triggered cache drift — every step + * before the accept is consistent, and `acceptAllChanges()` itself corrupted the maintained + * `.delta`. Shape: a committed `strong` run * ('a', present since the very first render) with a plain char ('b') right after it; a suggested * `strong` on 'b' with the SAME key AND the SAME value as the committed run; accept-all. The * accept commits the suggestion to base, and a fresh deep render shows one fully-committed @@ -1860,12 +1873,16 @@ export const testRdtAcceptAllOfSameValueSuggestedFormatCacheDrift = () => { } /** - * Currently-failing repro of the documented reject-side provenance residual (pre-existing; - * lifecycle-fuzzing class P1, 45 seeds; minimized from the 9-op seed-132 representative to - * 4 ops): a suggested strong on 'a' under a base strong over all of 'ab' — same key, with the - * base range extending strictly *past* the suggested span — then rejectAllChanges. The reject - * clears the suggestion, but the change render leaves the trailing 'b' still carrying the - * suggested-format attribution in the cache; the fresh render shows it unattributed: + * Regression pin — the long-documented reject-side provenance residual, fixed by the same + * `currentFormatsAttributed` provenance tracking as the accept class above (the failing render + * is the sugg-doc EVENT render — the reject's heal render receives an empty change set — and + * the deleted suggestion marker re-exposes the enclosing *committed* base strong, which the old + * `currFormatVal == null` guard could not see). Lifecycle-fuzzing class P1, 45 seeds; minimized + * from the 9-op seed-132 representative to 4 ops: a suggested strong on 'a' under a base strong + * over all of 'ab' — same key, with the base range extending strictly *past* the suggested + * span — then rejectAllChanges. The reject clears the suggestion, but the change render left + * the trailing 'b' still carrying the suggested-format attribution in the cache; the fresh + * render shows it unattributed: * * cached: a(strong) | b(strong, {format:{strong:[]}} ← STALE) * fresh : ab(strong)