1162 add caching for the outline and structure for the suggestions#23182
Conversation
Coverage Report for CI Build 0Coverage decreased (-0.02%) to 53.484%Details
Uncovered ChangesCoverage Regressions3 previously-covered lines in 3 files lost coverage.
Coverage Stats💛 - Coveralls |
There was a problem hiding this comment.
Pull request overview
This PR introduces outline caching for AI Content Planner suggestions (so revisiting the same suggestion can restore the previously fetched/edited outline without re-requesting it) and refactors the Content Outline modal UI into smaller components.
Changes:
- Add
indexto content suggestions and use it as the cache key for storing/restoring outlines. - Add outline cache state + actions to the content-outline store and wire cache restore into
useFetchContentOutline. - Refactor the Content Outline modal by extracting
ContentOutlineModalContent,StructureRow, andCategorySectioninto separate components and updating tests accordingly.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/js/src/ai-content-planner/store/content-suggestions.js | Adds index to each suggestion during response validation for stable cache addressing. |
| packages/js/src/ai-content-planner/store/content-outline.js | Adds outline cache state/actions and normalizes outline sections into { heading, id, contentNotes }. |
| packages/js/src/ai-content-planner/hooks/use-fetch-content-outline.js | Restores outline from cache when available, otherwise fetches from API. |
| packages/js/src/ai-content-planner/hooks/use-draggable-structure.js | Updates structure handling to use the normalized outline items directly. |
| packages/js/src/ai-content-planner/hooks/use-apply-outline.js | Switches to applying blocks from the edited structure directly. |
| packages/js/src/ai-content-planner/helpers/build-blocks-from-outline.js | Updates outline-to-blocks builder to use normalized section fields. |
| packages/js/src/ai-content-planner/containers/content-outline-modal.js | Persists outline edits into cache when navigating back to suggestions. |
| packages/js/src/ai-content-planner/components/content-outline-modal.js | Refactors modal to delegate body/footer to ContentOutlineModalContent. |
| packages/js/src/ai-content-planner/components/content-outline-modal-content.js | New component containing the editable form + draggable structure + retry behavior. |
| packages/js/src/ai-content-planner/components/structure-row.js | New extracted draggable structure row component. |
| packages/js/src/ai-content-planner/components/category-section.js | New extracted category toggle/summary component. |
| packages/js/src/ai-content-planner/constants.js | Documents the new Suggestion.index field used for caching. |
| packages/js/tests/ai-content-planner/store/suggestions.test.js | Updates fixtures to include index. |
| packages/js/tests/ai-content-planner/hooks/use-draggable-structure.test.js | Updates tests for normalized outline shape (heading instead of title/subheading_text). |
| packages/js/tests/ai-content-planner/components/feature-modal.test.js | Updates suggestion fixture to include index. |
| packages/js/tests/ai-content-planner/components/content-outline-modal.test.js | Updates mocks/expectations around retry (now calls fetchContentOutline). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Also, I followed these testing steps:
On the "Content suggestions" overview, the title and the meta description don't reflect my changes from the previous step. Is this expected? Screen.Recording.2026-04-22.at.10.36.34.mov |
…ion' into 1162-add-caching-for-the-outline-and-structure-for-the-suggestions
…ion' into 1162-add-caching-for-the-outline-and-structure-for-the-suggestions
|
A merge conflict has been detected for the proposed code changes in this PR. Please resolve the conflict by either rebasing the PR or merging in changes from the base branch. |
…ion' into 1162-add-caching-for-the-outline-and-structure-for-the-suggestions
This captures the selector function itself from useSelect, not a value. This works, but the intent is non-reactive imperative access.
|
A merge conflict has been detected for the proposed code changes in this PR. Please resolve the conflict by either rebasing the PR or merging in changes from the base branch. |
…ion' into 1162-add-caching-for-the-outline-and-structure-for-the-suggestions
pls78
left a comment
There was a problem hiding this comment.
I'll leave two Claude's findings that I think are worth looking into:
- id:
suggestion-${index}is positional → fragile cache key (store/content-suggestions.js:74)
The suggestion ID is just its position in the response. If the user re-fetches suggestions and the new list is shorter, longer, or reordered, "suggestion-0" maps to a different suggestion than what's in the cache. The reset on FETCH_CONTENT_SUGGESTIONS/request mitigates it (cache is wiped on re-fetch), but the safety relies entirely on that one reducer line. A content-derived id (e.g. hash of title + keyphrase) would be far more robust and would also let the cache survive reloads if you ever want that. At minimum, add a comment on the id field stating the invariant.
- Focus management regression in
OutlineModalContent(components/outline-modal-content.js:127-131)
useEffect( () => {
if ( isLoading && closeButtonRef?.current ) {
closeButtonRef.current.focus();
}
}, [ isLoading, closeButtonRef ] );
Previously, focus was set to the close button when the panel became active, ensuring SR users got the dialog context. Now focus is set only while loading, meaning when loading transitions to success, focus is not moved to the loaded content — and if a user tabs away during the loading state, focus may not be re-asserted. Was this intentional? If the goal was "focus when the modal first opens," tying it to isLoading only kind-of accomplishes that and breaks the success-direct path (cache hit → never loading → never focused).
I'd give prio to the second one, honestly.
Moreover, there's a mixed used of handleEvent and onEvent for callback names: I'd stick with HandleEvent.
|
A merge conflict has been detected for the proposed code changes in this PR. Please resolve the conflict by either rebasing the PR or merging in changes from the base branch. |
…62-add-caching-for-the-outline-and-structure-for-the-suggestions
|
Acc: ✅ |
Focus the close button on mount and again when loading completes, so SR users get dialog context immediately after clicking a suggestion and again once the skeleton resolves to real content.
Context
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
get_outline.get_outline.Relevant test scenarios
Test instructions for QA when the code is in the RC
QA can test this PR by following these steps:
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
Other environments
[shopify-seo], added test instructions for Shopify and attached theShopifylabel to this PR.[yoast-doc-extension], added test instructions for Yoast SEO for Google Docs and attached theGoogle Docs Add-onlabel to this PR.Documentation
Quality assurance
grunt build:imagesand commited the results, if my PR introduces new images or SVGs.Innovation
innovationlabel.Fixes https://github.com/Yoast/reserved-tasks/issues/1162