Skip to content

Suggest mode 2/9: suggestion storage, REST controller, and provider - #80428

Open
adamsilverstein wants to merge 67 commits into
suggest/intentfrom
suggest/data
Open

adamsilverstein wants to merge 67 commits into
suggest/intentfrom
suggest/data

Conversation

@adamsilverstein

@adamsilverstein adamsilverstein commented Jul 17, 2026 •

Copy link
Copy Markdown
Member

Part of #73411

What's in this PR

Adds the comment-meta storage backbone: _wp_suggestion payload meta and
sanitization (64KB cap, KSES on block snapshots), the 7.1 REST comment
controller subclass scoped to suggestion lifecycle updates, the
comment-meta SuggestionsProvider (create/update/delete, apply/reject for
attribute and structural operations, schema versioning + migration), the
pending-suggestion overlay store with its debounced write queue, and the
auto-save loop that persists overlay entries as note comments. Inline
(marker-based) operations are added by a later layer. Nothing captures
suggestions yet - that starts in the next layer.

Diagram

There is no UI in this layer. It is the write path that turns an edit into a stored suggestion, plus the read path back out:

How a suggestion gets stored: block edit to overlay to autosave to provider to REST to a note comment carrying _wp_suggestion meta

Testing

This is one layer of the stack. To exercise the whole feature, #78994 bundles every layer into one branch and builds it in Playground:

Test in WordPress Playground

👉 https://playground.wordpress.net/gutenberg.html?pr=78994

Enable Gutenberg > Experiments > Collaboration > Suggestion Mode, then follow the walkthrough in #73411, which also explains how to review the stack layer by layer.


Suggest mode stack

This rebuilds the manually-stacked Suggest mode work (#73411) as a GitHub Stack of 9 small, independently reviewable PRs, each building on the one below it:

  1. #80427 - editor intent (edit/suggest/view) + experiment gate
  2. #80428 - suggestion storage, REST controller, provider
  3. #80429 - block-level capture (attribute + structural)
  4. #80430 - inline marker primitive
  5. #80431 - inline suggestion operations
  6. #80432 - review UI (Apply/Reject sidebar + summary)
  7. #80433 - inline live wiring
  8. #82047 - architecture documentation
  9. #82048 - end-to-end test suite

Each follow up fix now sits in the layer that owns the code it changes, rather than piling onto the top of the stack. The whole feature can be exercised end-to-end via the combined testing branch #78994 (Playground). Behind the "Suggestion Mode" experiment (Gutenberg > Experiments).


AI Use

Code and description both written with 🤖 Claude Code. I will review and test.

Adds the comment-meta storage backbone: _wp_suggestion payload meta and
sanitization (64KB cap, KSES on block snapshots), the 7.1 REST comment
controller subclass scoped to suggestion lifecycle updates, the
comment-meta SuggestionsProvider (create/update/delete, apply/reject for
attribute and structural operations, schema versioning + migration), the
pending-suggestion overlay store with its debounced write queue, and the
auto-save loop that persists overlay entries as note comments. Inline
(marker-based) operations are added by a later layer. Nothing captures
suggestions yet - that starts in the next layer.
@github-actions

github-actions Bot commented Jul 17, 2026 •

Copy link
Copy Markdown

Size Change: +4.24 kB (+0.05%)

Total Size: 7.92 MB

📦 View Changed
Filename Size Change
build/scripts/editor/index.min.js 587 kB +4.24 kB (+0.73%)

compressed-size-action

@adamsilverstein adamsilverstein added No Core Sync Required Indicates that any changes do not need to be synced to WordPress Core [Type] Feature New feature to highlight in changelogs. labels Jul 17, 2026
# Conflicts:
#	tools/eslint/suppressions.json
@adamsilverstein
adamsilverstein marked this pull request as ready for review August 5, 2026 22:24
@github-actions

github-actions Bot commented Aug 5, 2026 •

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>
Co-authored-by: aduth <aduth@git.wordpress.org>
Co-authored-by: mikachan <mikachan@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

The auto-save scheduler never cleared its debounce timers when the intent
changed. Because the component is mounted on the experiment flag rather
than the intent, switching from Suggest to Edit or View left any armed
timer running, and it went on to POST a note for edits the user had just
walked away from - visible to collaborators as a deliberate suggestion.

Cancelling is not lossy: the overlay entry keeps its unsynced fingerprint,
so returning to Suggest reschedules the save.
… tree

Applying or rejecting a block-remove, block-insert-after, or block-move
suggestion dispatched the tree mutation first and only surfaced an error
notice if the save failed. Nothing rolled the tree back, so a failed save
left the editor diverged from the server: the block was gone (or moved)
while the note still read as pending, and for block-remove the marker went
with the block, leaving no way to act on the note again.

The attribute-set path solves this by snapshotting attributes and
restoring them. A structural rollback cannot be that faithful - position,
nested children, selection, and the overlay entry would all need
restoring - so await the lifecycle write first instead. The mutation only
runs once the decision is persisted, which is the same invariant for less
machinery.
@github-actions

github-actions Bot commented Aug 23, 2026 •

Copy link
Copy Markdown

Flaky tests detected in c05f47a.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/33117670591
📝 Reported tests:

refuses the drop and uploads nothing in /test/e2e/specs/editor/various/single-file-placeholder-drop.spec.js, passed after 1 failed attempt.
Error: expect(received).toHaveLength(expected)

Expected length: 0
Received length: 2
Received array:  [{"_links": {"about": [{"href": "http://localhost:8889/wp-json/wp/v2/types/attachment"}], "author": [{"embeddable": true, "href": "http://localhost:8889/wp-json/wp/v2/users/1"}], "collection": [{"href": "http://localhost:8889/wp-json/wp/v2/media"}], "curies": [{"href": "https://api.w.org/{rel}", "name": "wp", "templated": true}], "replies": [{"embeddable": true, "href": "http://localhost:8889/wp-json/wp/v2/comments?post=106"}], "self": [{"href": "http://localhost:8889/wp-json/wp/v2/media/106", "targetHints": {"allow": ["GET", "POST", "PUT", "PATCH", "DELETE"]}}], "wp:attached-to": [{"embeddable": true, "href": "http://localhost:8889/wp-json/wp/v2/posts/104", "id": 104, "post_type": "post"}]}, "alt_text": "", "author": 1, "caption": {"rendered": ""}, "class_list": ["post-106", "attachment", "type-attachment", "status-inherit", "hentry", "entry"], "comment_status": "open", "date": "2026-08-27T21:32:17", "date_gmt": "2026-08-27T21:32:17", "description": {"rendered": "<p class=\"attachment\"><a href='http://localhost:8889/wp-content/uploads/2026/08/10x10_e2e_test_image_z9T8jK-1.png'><img loading=\"lazy\" decoding=\"async\" width=\"10\" height=\"10\" src=\"http://localhost:8889/wp-content/uploads/2026/08/10x10_e2e_test_image_z9T8jK-1.png\" class=\"attachment-medium size-medium\" alt=\"\" style=\"width:100%;height:100%;max-width:10px;\" /></a></p>
"}, "featured_media": 0, "filename": "10x10_e2e_test_image_z9T8jK-1.png", "filesize": 80, "guid": {"rendered": "http://localhost:8889/wp-content/uploads/2026/08/10x10_e2e_test_image_z9T8jK-1.png"}, "id": 106, "link": "http://localhost:8889/?attachment_id=106", "media_details": {"file": "2026/08/10x10_e2e_test_image_z9T8jK-1.png", "filesize": 80, "height": 10, "image_meta": {"alt": "", "aperture": "0", "camera": "", "caption": "", "copyright": "", "created_timestamp": "0", "credit": "", "focal_length": "0", "iso": "0", "keywords": [], "orientation": "0", "shutter_speed": "0", "title": ""}, "sizes": {}, "width": 10}, "media_type": "image", "meta": [], "mime_type": "image/png", "modified": "2026-08-27T21:32:17", "modified_gmt": "2026-08-27T21:32:17", "ping_status": "closed", "post": 104, "slug": "10x10_e2e_test_image_z9t8jk-2", "source_url": "http://localhost:8889/wp-content/uploads/2026/08/10x10_e2e_test_image_z9T8jK-1.png", "status": "inherit", "template": "", "title": {"rendered": "10x10_e2e_test_image_z9T8jK"}, "type": "attachment"}, {"_links": {"about": [{"href": "http://localhost:8889/wp-json/wp/v2/types/attachment"}], "author": [{"embeddable": true, "href": "http://localhost:8889/wp-json/wp/v2/users/1"}], "collection": [{"href": "http://localhost:8889/wp-json/wp/v2/media"}], "curies": [{"href": "https://api.w.org/{rel}", "name": "wp", "templated": true}], "replies": [{"embeddable": true, "href": "http://localhost:8889/wp-json/wp/v2/comments?post=105"}], "self": [{"href": "http://localhost:8889/wp-json/wp/v2/media/105", "targetHints": {"allow": ["GET", "POST", "PUT", "PATCH", "DELETE"]}}], "wp:attached-to": [{"embeddable": true, "href": "http://localhost:8889/wp-json/wp/v2/posts/104", "id": 104, "post_type": "post"}]}, "alt_text": "", "author": 1, "caption": {"rendered": ""}, "class_list": ["post-105", "attachment", "type-attachment", "status-inherit", "hentry", "entry"], "comment_status": "open", "date": "2026-08-27T21:32:17", "date_gmt": "2026-08-27T21:32:17", "description": {"rendered": "<p class=\"attachment\"><a href='http://localhost:8889/wp-content/uploads/2026/08/10x10_e2e_test_image_z9T8jK.png'><img loading=\"lazy\" decoding=\"async\" width=\"10\" height=\"10\" src=\"http://localhost:8889/wp-content/uploads/2026/08/10x10_e2e_test_image_z9T8jK.png\" class=\"attachment-medium size-medium\" alt=\"\" style=\"width:100%;height:100%;max-width:10px;\" /></a></p>
"}, "featured_media": 0, "filename": "10x10_e2e_test_image_z9T8jK.png", "filesize": 80, "guid": {"rendered": "http://localhost:8889/wp-content/uploads/2026/08/10x10_e2e_test_image_z9T8jK.png"}, "id": 105, "link": "http://localhost:8889/?attachment_id=105", "media_details": {"file": "2026/08/10x10_e2e_test_image_z9T8jK.png", "filesize": 80, "height": 10, "image_meta": {"alt": "", "aperture": "0", "camera": "", "caption": "", "copyright": "", "created_timestamp": "0", "credit": "", "focal_length": "0", "iso": "0", "keywords": [], "orientation": "0", "shutter_speed": "0", "title": ""}, "sizes": {}, "width": 10}, "media_type": "image", "meta": [], "mime_type": "image/png", "modified": "2026-08-27T21:32:17", "modified_gmt": "2026-08-27T21:32:17", "ping_status": "closed", "post": 104, "slug": "10x10_e2e_test_image_z9t8jk", "source_url": "http://localhost:8889/wp-content/uploads/2026/08/10x10_e2e_test_image_z9T8jK.png", "status": "inherit", "template": "", "title": {"rendered": "10x10_e2e_test_image_z9T8jK"}, "type": "attachment"}]
    at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/various/single-file-placeholder-drop.spec.js:71:45

Rename the files this PR adds from .js to .ts/.tsx and add types so the
package's strict type check covers them, per the repo convention that new
files are authored in TypeScript. No behavior changes: the overlay entry,
context value, reducer action, and suggestion payload shapes are now
explicit interfaces, and eslint suppression paths follow the renames.
The restore used the recorded fromParentClientId as its destination,
but client ids are regenerated on every parse, so rejecting even a
same-parent move inside a Group after a reload targeted a parent that
no longer exists. A same-parent move now restores within the block's
live parent; a cross-parent move uses the recorded parent only while it
still exists.

Claude-Session: https://claude.ai/code/session_01EmSXGAGtuLnk3UEe5kXPSU
Leaving the suggest intent cleared every debounce timer, so a proposal
made moments before switching to Editing lived only in React state and
was lost on save and reload. Switching intents is a normal part of the
review workflow, so enqueue the pending saves immediately instead.

Claude-Session: https://claude.ai/code/session_01EmSXGAGtuLnk3UEe5kXPSU
@adamsilverstein

Copy link
Copy Markdown
Member Author

Claude worked through the latest review of this layer, three fixes landed:

  • Attribute rejects clear their overlay (e1998ee). Rejecting an attribute-only suggestion left the overlay entry in place, so the rejected value kept rendering and fed the next proposal. It now uses the guarded clearOverlayForComment(), so an entry a newer suggestion already took over is kept.
  • Nested move rejects after a reload (ceb3997). The restore targeted the recorded fromParentClientId, which is a session-local id. A same-parent move now restores within the block's live parent, and a cross-parent move uses the recorded parent only while it still exists. That now matches what the architecture doc already describes.
  • Leaving Suggesting flushes pending saves (e4fdd2f). Switching to Editing mid-debounce used to cancel the save, so the proposal only lived in React state until a reload dropped it. The pending saves are now enqueued immediately.

Each fix has a unit test that fails without it. This push also merges the latest suggest/intent.

adamsilverstein and others added 8 commits September 28, 2026 23:24
Structured `after` values such as table rows and snapshot attributes
were stored unfiltered for users without unfiltered_html. Reuse core's
filter_block_kses_value() so they get the same walk parsed block
attributes get.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YHz7zkCC2S8crriSWsYPDk
The overlay context bundled every block's entries with its stable
callbacks, and hasOverlay changed identity with entries, so each
overlay write re-rendered every consumer. Entries now live in a small
external store; a stable actions context and useOverlayEntry(), a
per-clientId subscription, let per-block consumers opt out of other
blocks' writes. useSuggestionOverlay() is unchanged. The orphan prune
looks up each entry instead of building a full-tree Set.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YHz7zkCC2S8crriSWsYPDk
Every overlay change re-fingerprinted every entry and restarted every
block's timer, so steady typing in one block postponed another block's
save. Only entries whose object changed are rescheduled now. Pending
saves are flushed on unmount instead of dropped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YHz7zkCC2S8crriSWsYPDk
Lets the store interceptor skip store fires that leave the block tree
unchanged without stranding a bypass token that the old per-fire walk
would have consumed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YHz7zkCC2S8crriSWsYPDk
Comment thread packages/editor/src/components/suggestion-mode/provider.ts Outdated
adamsilverstein and others added 4 commits September 30, 2026 16:01
'snackbar' is a notice type, not a status, and the 'as any' cast was
hiding the type error. Use 'success' for the applied and rejected
snackbars; the options already set the snackbar type.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016qzrCziUUHcLxYeMHysRcQ

@mikachan mikachan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for splitting this into a stack, @adamsilverstein. I've had Claude take a look at this and I've left some inline comments with the feedback.

Comment thread packages/editor/src/components/suggestion-mode/auto-save.ts
Comment thread packages/editor/src/components/suggestion-mode/auto-save.ts Outdated
Comment thread lib/compat/wordpress-7.1/class-gutenberg-rest-comment-controller-7-1.php Outdated
Comment thread packages/editor/src/components/suggestion-mode/provider.ts Outdated
adamsilverstein and others added 8 commits October 1, 2026 16:05
When an overlay returns to baseline, auto-save trashes the note but left
its id in the block's `metadata.noteId`. That attribute is post content,
so the post saved a reference to a trashed comment that downstream
`getNoteIdsFromMetadata` consumers would try to resolve.

`deleteSuggestion` now takes the block's clientId and removes the id
once the trash succeeds, outside undo history like the link written on
create.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E9vmiAPMMg3s4HNJxXAYxy
The attribute apply path cleared the overlay entry before saving the
decision. On a server rejection the catch rolled the attributes back,
but the suggester's overlay entry was gone for good, with no way to
recover the still-pending proposal.

Clear it only after the save succeeds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E9vmiAPMMg3s4HNJxXAYxy
The subclass let users with `edit_post` on the parent post update
lifecycle-only fields on a note. Core already admits exactly those
users: `check_edit_permission()` falls through to `edit_comment`, which
`map_meta_cap()` resolves to `edit_post` on the comment's parent post.
The shortcut granted nothing, and it skipped any `map_meta_cap` filter a
site adds for `edit_comment`.

Remove it and the reflection tests for its allowlist helper. The REST
dispatch tests for an editor applying and a subscriber being refused
still pass through core's check. Also correct the registration comment:
core registers its routes at priority 99, so this subclass registers
first, not after.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E9vmiAPMMg3s4HNJxXAYxy
Trunk folded the floating notes sidebar into the canvas margin and dropped
the SIDEBARS list, so the post-submit sidebar switch now checks against
ALL_NOTES_SIDEBAR directly.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017NSEHmsqyXW8Yf62bJ3ckf
@adamsilverstein

Copy link
Copy Markdown
Member Author

Thanks for the review and feedback @mikachan! I'll go thru these to see what remains actionable!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Notes Phase 3 of the Gutenberg roadmap around block commenting No Core Sync Required Indicates that any changes do not need to be synced to WordPress Core [Package] Editor /packages/editor [Type] Feature New feature to highlight in changelogs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants