Skip to content

Commit 63902f7

Browse files
committed
Jetpack AI: guard every model-supplied list the review card indexes
affected_blocks was not the only one. conflict.positions and edit.supported_by_reviewers are typed as required by the tool schema, but the provider drops `strict`, so the model can omit either. Both are indexed unguarded, so either one throws the same TypeError and unmounts the card. A single toList() helper replaces the inline guard, so all three sites read the same way and a new one is a one-liner rather than a guard to remember. The three fields are now optional on the interface, which is what the payload actually promises.
1 parent a94b8fb commit 63902f7

2 files changed

Lines changed: 53 additions & 8 deletions

File tree

packages/jetpack-ai-sidebar/src/components/ai-editorial-review.test.tsx

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,43 @@ describe( 'AiEditorialReview — smoke render', () => {
465465
expect( mockedRecordTracksEvent ).not.toHaveBeenCalled();
466466
} );
467467

468+
it( 'renders a conflict that omits positions', () => {
469+
render(
470+
<AiEditorialReview
471+
{ ...basePayload( {
472+
conflicts: [
473+
{
474+
subject: 'Tone of the opening',
475+
guideline_anchor: null,
476+
recommended_resolution: 'Use neutral phrasing.',
477+
},
478+
] as any,
479+
} ) }
480+
/>
481+
);
482+
483+
expect( screen.getByText( 'Tone of the opening' ) ).toBeInTheDocument();
484+
} );
485+
486+
it( 'renders a suggested edit that omits supported_by_reviewers', () => {
487+
render(
488+
<AiEditorialReview
489+
{ ...basePayload( {
490+
suggested_edits: [
491+
{
492+
block_index: 1,
493+
current_text: 'voted last Tuesday',
494+
suggested_text: 'voted on Tuesday',
495+
rationale: 'Concise.',
496+
},
497+
] as any,
498+
} ) }
499+
/>
500+
);
501+
502+
expect( screen.getByText( 'Concise.' ) ).toBeInTheDocument();
503+
} );
504+
468505
it( 'renders an implication that omits affected_blocks', () => {
469506
// The tool schema marks affected_blocks required but the tool is not strict,
470507
// so the model can omit it. Indexing it directly used to blank the card.

packages/jetpack-ai-sidebar/src/components/ai-editorial-review.tsx

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ interface CandidateResolution {
6464

6565
interface Conflict {
6666
subject: string;
67-
positions: ReviewerPosition[];
67+
positions?: ReviewerPosition[];
6868
guideline_anchor: string | null;
6969
recommended_resolution: string;
7070
candidate_resolutions?: CandidateResolution[];
@@ -84,7 +84,7 @@ interface SuggestedEdit {
8484
suggested_text: string;
8585
suggested_text_html?: string;
8686
rationale: string;
87-
supported_by_reviewers: string[];
87+
supported_by_reviewers?: string[];
8888
requires_manual?: boolean;
8989
/** Optional short editorial category for the card badge (e.g. "Tone"). */
9090
feedback_category?: string;
@@ -198,6 +198,15 @@ function formatRelativeTime( timestamp: number ): string {
198198
);
199199
}
200200

201+
/**
202+
* Model-supplied list fields are typed as required by the tool schema, but the
203+
* provider drops `strict`, so any of them can arrive missing or malformed.
204+
* Reading `.length` or `.map` off one that did unmounts the whole card.
205+
*/
206+
function toList< T >( value: T[] | undefined ): T[] {
207+
return Array.isArray( value ) ? value : [];
208+
}
209+
201210
function getGuidelineCategoryLabel( category: GuidelineViolation[ 'category' ] ): string {
202211
switch ( category ) {
203212
case 'site':
@@ -1282,7 +1291,7 @@ export default function AiEditorialReview( {
12821291
) }
12831292
</header>
12841293
<ul className="jetpack-ai-editorial-review__positions">
1285-
{ conflict.positions.map( ( pos, j ) => (
1294+
{ toList( conflict.positions ).map( ( pos, j ) => (
12861295
<li
12871296
className="jetpack-ai-editorial-review__position"
12881297
key={ `pos-${ i }-${ j }` }
@@ -1391,9 +1400,7 @@ export default function AiEditorialReview( {
13911400
<ul>
13921401
{ implications.map( ( imp, i ) => {
13931402
// A non-strict payload can omit affected_blocks.
1394-
const affectedBlocks = Array.isArray( imp.affected_blocks )
1395-
? imp.affected_blocks
1396-
: [];
1403+
const affectedBlocks = toList( imp.affected_blocks );
13971404
return (
13981405
<li key={ `imp-${ i }` }>
13991406
<strong>{ imp.change }</strong>{ imp.implies }
@@ -1515,11 +1522,12 @@ export default function AiEditorialReview( {
15151522
element: 'text',
15161523
} );
15171524
}
1525+
const supportedByReviewers = toList( edit.supported_by_reviewers );
15181526
const footer =
1519-
edit.supported_by_reviewers.length > 0 ? (
1527+
supportedByReviewers.length > 0 ? (
15201528
<p className="jetpack-ai-editorial-review__reviewers">
15211529
{ __( 'Requested by:', __i18n_text_domain__ ) }{ ' ' }
1522-
{ edit.supported_by_reviewers.map( ( r, j ) => (
1530+
{ supportedByReviewers.map( ( r, j ) => (
15231531
<span key={ `edit-${ i }-rev-${ j }` }>
15241532
{ j > 0 && ' ' }
15251533
<ReviewerChip

0 commit comments

Comments
 (0)