Suggest mode: core subsystem (intent modes, overlay capture, structural + attribute suggestions)#79713
Open
adamsilverstein wants to merge 30 commits into
Open
Suggest mode: core subsystem (intent modes, overlay capture, structural + attribute suggestions)#79713adamsilverstein wants to merge 30 commits into
adamsilverstein wants to merge 30 commits into
Conversation
…al + attribute suggestions) Fresh-stack migration of the Suggest mode foundation onto current trunk, re-sliced from the suggest-mode feature branches into a single reviewable base. Includes the Edit/Suggest/View intent switcher, the in-memory overlay capture + store interceptor, attribute and structural (block remove/insert/ move) suggestions with the v2 declarative payload schema, the suggestion diff/summary, auto-save, the collaboration-sidebar Apply/Reject controls, and the REST/PHP backend (note-suggestion controller + render strip). The inline (in-text marker) layer is split into a separate stacked branch. This is a behaviour-preserving migration; planned refactors (overlay-inline retirement, identity-token interceptor, structural op-descriptor table) follow as separate PRs. Replaces the prior 16-deep stack (see #73411).
Adds a 'Suggestion Mode' experiment (id: gutenberg-suggestion-mode) under a new Collaboration group on the Experiments screen, so the whole feature can be enabled experimentally before final approval. - Registers the experiment in lib/experimental/experiments/load.php. - Bridges window.__experimentalSuggestionMode via gutenberg_enable_experiments(). - Gates the PHP requires (block-suggestions.php render strip + the note- suggestion REST controller) in lib/load.php behind the experiment. - Gates the JS surfaces: the IntentSwitcher menu entry, the suggestion provider mounts (store interceptor + auto-save), and the intent-switch keyboard shortcut handlers, so none activate unless the experiment is on. block-comments.php (Notes) stays ungated; it is shipped, not experimental.
This was referenced Jun 30, 2026
|
Size Change: +14.5 kB (+0.19%) Total Size: 7.69 MB 📦 View Changed
|
- Always require the suggestion REST controller + front-end marker strip instead of gating them on the experiment. Both are inert without suggestion data (only the experiment-gated editor UI creates it), and an always-on strip ensures un-accepted markers never reach the front end. This also keeps the PHP REST controller unit tests working (the class is always defined). The user-facing feature stays gated in JS. - Enable the Suggestion Mode experiment in the intent-switcher and suggestion- mode e2e specs (they drive the now-gated UI). - Drop collab-sidebar/test/hooks.js: it tested the feature's inline-markers refactor of the Notes hooks, which was dropped in favour of trunk's self-contained Notes implementation.
18e39ea to
bb166ee
Compare
… intent menu Rename the more-menu Mode picker entries from the bare verbs Edit/Suggest/ View to their progressive forms Editing/Suggesting/Viewing so each reads as the state the editor is currently in. Update the e2e specs that match those accessible names.
The add/delete/style "golden path" tests assert per-word inline marks (<ins class="has-suggestion-addition"> / <del class="has-suggestion-deletion">) that are produced by the inline-suggestions layer, not the core overlay. On the core branch alone they cannot pass, so move them to the inline spec and leave the core spec covering only core suggestion behavior: snackbar announcements, auto-save, the store interceptor, and the empty-inserted-block guard.
This was referenced Jun 30, 2026
rejectSuggestion for a block-move op passed the ORIGINAL parent as both fromRootClientId and toRootClientId to moveBlockToPosition. After a cross-parent move the block lives in the NEW parent, so the reducer's index lookup in the claimed from-root missed the block and left state unchanged - while the note was still marked rejected, desyncing the canvas from the suggestion lifecycle. Pass the block's CURRENT root (getBlockRootClientId) as the from-root and the captured fromParentClientId as the to-root, so the reducer finds the block where it actually is and restores it to where it came from. Add hook-level reject tests covering both the cross-parent case (which reproduced the no-op before the fix) and the same-parent case.
…params is_suggestion_lifecycle_update() inspected only the JSON or form body, but core's update_item reads fields like content from the MERGED param view (JSON > POST > GET > URL). A request like PUT /wp/v2/comments/<id>?content=rewritten with a lifecycle-only body passed the allowlist yet rewrote the note under the edit_post shortcut. Build the inspected key set from the union of every client-supplied channel (URL, query, body, JSON) so smuggled query parameters are caught, while ignoring keys the client didn't meaningfully send: the route's id, REST meta-parameters (_locale rides on every api-fetch request), and response-shaping context. Server-injected schema defaults (post, parent) are excluded by construction since only explicit channels are unioned. Also rewrite the class and method docblocks, which claimed the allowlist ensures the apply/reject path 'can never be used to rewrite another user's note'. That was false: core maps edit_comment to edit_post on the comment's parent post, so post editors already hold full note-edit permission through the fallback. The docblocks now state the shortcut only widens access for lifecycle-field-only updates. Add tests proving a query-param content rewrite does not take the shortcut and that _locale does not disqualify legitimate updates.
Gating was inconsistent across surfaces: the overlay block filters were registered at module scope for ALL users (adding per-block subscriptions even with the experiment off), SuggestionOverlayProvider mounted unconditionally, the intent keyboard-shortcut handlers checked only the experiment flag and skipped the editor.notes post-type-support check the intent menu enforces, and the intent shortcuts were registered unconditionally so they appeared in the Keyboard Shortcuts help modal with the experiment off. Introduce a shared gate module with two predicates: - isSuggestionModeEnabled(): the experiment flag. Safe at module scope because PHP writes the flag before any editor script evaluates. Used to gate filter registration, the overlay provider mount, shortcut registration, and the intent menu. - useCanSuggest(): flag AND editor.notes post-type support, reusing the same checkSupport helper PostTypeSupportCheck uses so the shortcut and menu gates can't diverge. Used by the intent shortcut handlers. With the experiment off the overlay context never mounts and consumers fall back to the inert context default.
setEditorIntent and getEditorIntent were added as new PUBLIC core/editor APIs backing an experiment - a forever compat commitment against the repo's convention that experimental surface ships behind the store's private API mechanism. Move them to private-actions.js / private-selectors.js (registered via registerPrivateActions / registerPrivateSelectors) and update every consumer to unlock(): the intent switcher, the keyboard shortcut handlers, the suggest-mode subsystem's intent reads, and the tests. use-block-editor-settings.js already read through unlock(). The reducer and state shape are unchanged, so persisted behavior is identical. Remove the two entries from the generated data docs by hand and note the private status in the editor CHANGELOG.
The store interceptor's programmatic writes - the revert that restores a block to its baseline, the pending-insert/move/remove marker writes, and the re-insert of a removed subtree - were plain persistent dispatches. Each one cut an undo level, so Ctrl+Z could undo a revert (re-applying the suggested change to real content) or strip a pending marker while the overlay still held the op, desyncing canvas and suggestion state. Dispatch __unstableMarkNextChangeAsNotPersistent() on the block-editor store immediately before each interceptor-originated dispatch - the established core pattern for programmatic writes. User edits themselves remain persistent; only the interceptor's bookkeeping is exempt. Add tests asserting the revert, the re-insert + marker write, and the pending-insert marker write leave isLastBlockChangePersistent() false.
The editor intent is implemented as a session-scoped reducer, but several places still described the abandoned preferences-store design: the store constants claimed the intent is persisted via @wordpress/preferences and survives reloads, the architecture doc said the intent is stored under core.editorIntent in the preferences store, and edit-post / edit-site registered a dead editorIntent preference default that nothing reads. Remove the dead preference defaults and reword the docs to describe the session-scoped reducer storage, so readers don't hunt for preference plumbing that doesn't exist. The selector/action docblocks were already corrected when the API moved to the private store surface.
- Cap the word-LCS input in the sidebar suggestion summary with the same MAX_DIFF_LENGTH fallback DiffForOperation uses, so a payload near the 64KB limit can't freeze the sidebar with an O(m*n) diff. - Preserve the ORIGINAL from-position when a block already carrying a pending-move marker is moved again; overwriting it with the intermediate position made Reject restore a spot that was itself only a pending suggestion. Covered by a new interceptor test. - Use sprintf() for the 'New block: %s' fallback label instead of a manual replace on the translated string. - Target the Accept button by class instead of aria-label in the suggestion header styles; the label is translated so the attribute selector silently stopped matching in non-English locales. - Compare against the EDITOR_INTENT_VIEW constant instead of the 'view' literal in use-block-editor-settings. - Attach the suggestion auto-save waitForResponse listener BEFORE the edit that starts the debounce in the e2e spec; attaching after races the response on slow CI and hangs the wait for its full timeout. - Correct a stale comment pointing at wordpress-6.9/block-comments.php; the payload size constant mirrors wordpress-7.1/block-suggestions.php. - Derive the Accept button's disabled state and its explanatory reason from one predicate (missing blockClientId counts as a missing target) so the two can't disagree.
# Conflicts: # packages/editor/CHANGELOG.md # packages/editor/src/components/provider/use-block-editor-settings.js
Match the intent switcher's Editing / Suggesting / Viewing labels and say plainly that other users apply or reject the proposed changes, dropping the trailing document-suggestion comparison.
|
Flaky tests detected in 39438d1. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/28844296542
|
…ertion suggestion Typing into a newly-appended empty paragraph produced TWO notes: an inline/overlay content suggestion plus an 'Insert block' one. The content path fired first, and its programmatic metadata.noteId write made the deferred empty block look modified, so the interceptor then registered a second, structural suggestion for the same action. A block that IS the suggestion has no 'before' state to preserve, so its edits now write through to the real attributes and stay part of the single block-insert suggestion: - overlay-context publishes the interceptor's empty-placeholder deferral (markDeferredInsertion / isDeferredInsertion) so the first edit into a deferred block can be recognized outside the interceptor. - The interceptor adopts (rather than overlay-diverts + reverts) attribute changes on blocks that are, or are nested inside, a pending insertion, and forgets a deferred block that is deleted before gaining content instead of proposing its removal. - The overlay HOC writes setAttributes through for deferred blocks and extends the pending-insert pass-through to descendants of an inserted block (children of a suggested-in Group). The inline suggestion keyboards gain matching guards in the stacked inline-markers branch.
…rtion wiring The deferred-insertion context callbacks are identity-stable, so the interceptor closes over them directly instead of the render-written ref pattern (frozen at its current count in eslint suppressions), and the HOC test captures the overlay handle from an effect rather than during render.
…component The SuggestionDiff inline diff preview was exported from the suggestion-mode barrel but never rendered anywhere in the subsystem - only its wordDiff engine is consumed (by the sidebar summary). Extract wordDiff and its caps into word-diff.js and drop the dead component, its tests, and its eslint suppression, so reviewers do not spend time on code the marker-based inline layer replaces. Also align the architecture doc and e2e header with what this layer actually renders (pending-treatment classes + sidebar summary; in-content rendering belongs to the inline-suggestions layer stacked on this PR).
Ports the friendlier Suggestion Mode experiment description (already live on try/suggest-mode-combined) onto the stack so the combined branch carries no stack-external changes.
The accept/reject half of the Suggest mode loop had no e2e coverage, and neither did the block-remove and block-move structural captures. Add a review-flows spec exercising the common flows end to end: deleting and moving blocks in Suggesting intent (pending treatment, move ghost, note summary), then accepting or rejecting each suggestion kind this layer ships (attribute-set, block-remove, block-insert-after, block-move) from the notes sidebar in Editing intent, asserting the canvas, the serialized post content, and the note's resolved state.
…s created When a new suggestion note is saved while a non-notes sidebar (e.g. post or block settings) is open, switch it to the All notes sidebar so the note is immediately visible. A closed sidebar stays closed, and an already-open notes sidebar is left alone.
…ng saves Structural suggestion state (metadata.suggestion markers, pending-insert blocks, a pending move's proposed order) previously lived only in the unsaved block tree: SuggestionSaveLock held the editor save/autosave locks while any of it existed, so the post stayed dirty with saving disabled - trapping the suggester behind a beforeunload warning - and a reload lost the pending state, orphaning its note. Persist the pending state into post_content instead, the structural counterpart of inline markers living in content: - Remove SuggestionSaveLock; saving and autosaving stay available. - Hide un-accepted insertions at render time via a type-aware render_block strip (gutenberg_strip_pending_structural_suggestions), mirroring the inline add-marker strip. pending-remove and pending-move blocks render (their content is real until accepted). - Document the new model plus its limitations (pending-move front-end order, cross-parent move anchors after reload).
Pin the persistence model: a structural suggestion leaves Save draft available and the editor clean after saving; a pending move/remove survives a reload with its note linked and can still be rejected or accepted; an un-accepted insertion never renders on the front end.
Order-diffing alone cannot attribute an adjacent swap: moving B above A produces the same before/after order as moving A below B, and the LCS tie-break could pick either reading. Moving a block up tagged the block it passed instead of the moved one, put the origin ghost at the wrong slot, and each further hop tagged a different displaced sibling - piling up a note per hop instead of updating one. Prefer the reading in which the selected blocks are the movers (a block moved via toolbar, keyboard, or drag stays selected), falling back to the LCS heuristic when the selection does not explain the reorder. Also stop re-capturing a rejected move: reject now batches the marker-clear with the restoring moveBlockToPosition, and the interceptor recognizes that shape (marker in the previous tick, gone on the live block) as the suggestion landing - previously, rejecting a move while in Suggesting intent immediately spawned a phantom move suggestion.
Ctrl+Z right after making a suggestion previously mangled it: the store interceptor re-captured the undo-induced change as a brand-new suggestion, attribute suggestions (held in the overlay, invisible to undo history) ignored undo entirely, and the withdrawn suggestion's note survived as an orphan with nothing left to accept or reject. Make undo suggestion-aware while Suggest intent is active: - A new SuggestionUndoGuard wraps the core-data undo/redo actions. It cancels the most recently captured attribute suggestion by reverting its overlay entry to baseline, withdraws a pending move or insertion the way Reject restores the block (as history-ignored writes, since the async note linkage otherwise resurrects the marker when the block survives the history transaction), and otherwise arms an adoption token so the store interceptor adopts the undo result as the new capture baseline instead of re-capturing it. Suggested removals revert cleanly through the real undo stack because the history transaction replaces the re-inserted block wholesale. - A new SuggestionNoteGC trashes a pending note once an anchor it has observed (structural marker + noteId linkage, or overlay entry) has disappeared, closing the orphaned-note gap documented in suggestions.md. Apply/reject races are excluded via a decisions-in- flight registry in the provider and a pending-status recheck. - The provider's noteId linkage write is marked history: 'ignore' — it is system bookkeeping, and as a tracked write it landed past any history coalescing window and ate the first undo as its own stack item. See the capture-sequence stamps in overlay-context.js for how the guard orders overlay-held suggestions against inline marker captures.
…stions Pin the withdrawal contract for cmd/ctrl+Z after each capture kind the core layer owns: a block removal sheds its pending treatment without spawning new suggestions, an insertion is taken back as one unit without being re-captured as a removal, a move restores the original order and clears its ghost, and a pending attribute change is cancelled outright. In every case the suggestion's note leaves the sidebar — a note whose proposed change no longer exists has nothing left to accept or reject.
Cover the remaining attribute kind from the #73411 testing instructions: text alignment is a block support writing style.typography.textAlign straight to the store, so the store interceptor captures it as an attribute suggestion; accepting lands the alignment in the post content. The pending treatment is the only visible cue while pending — the support's useBlockProps hook reads the (reverted) store attributes, so the proposed alignment renders only once accepted.
# Conflicts: # packages/editor/src/store/constants.ts
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.
What
Fresh-stack replacement for the Suggest mode core, re-sliced from the prior
16-deep stack onto current
trunkas a single reviewable base. Rationale andmap: #73411.
This is the foundation the inline marker layer (#79714) stacks on. It contains
everything except the inline
<mark>marker layer:an overlay; structural edits apply-and-tag on the live block). The overlay is
a capture/store mechanism only — it never renders inline content diffs; all
in-content rendering of text changes lives in the stacked marker layer
(Suggest mode: inline text and formatting suggestions via markers #79714).
declarative
operations[]payload schema and the move-ghost placeholder.word-diffengine), auto-save, and thecollaboration-sidebar Apply / Reject controls.
experiment (Settings → Experiments → Collaboration), off by default.
Fixes #73411, #77867, #73410
Migrate-first
Behaviour-preserving migration of code that previously lived across #77403–#78412.
Remaining planned refactors (e.g. a structural op-descriptor table) land as
separate follow-ups on this branch (see #73411).
Testing
Enable Settings → Experiments → Collaboration → Suggestion Mode, then use the
Edit/Suggest/View switcher in the post editor. Inline text suggestions come with
#79714. Combined Playground preview: #78994.
Replaces #77403, #77404, #77405, #77406, #78351, #78352, #78353, #78308, #77967,
#77968, #77970, #77971, #77973, #77978, #77979, #78412.