Skip to content

Repo Gardening: Add AI-based check for user-facing PR changes#46701

Merged
jeherve merged 30 commits intotrunkfrom
add/ai-ui-changes-check
Jan 29, 2026
Merged

Repo Gardening: Add AI-based check for user-facing PR changes#46701
jeherve merged 30 commits intotrunkfrom
add/ai-ui-changes-check

Conversation

@jeherve
Copy link
Copy Markdown
Member

@jeherve jeherve commented Jan 21, 2026

Fixes MONOREP-325

Proposed changes:

This PR adds a new checkIfDocsNeeded task to the repo-gardening action that uses AI to automatically detect user-facing changes in PRs. When detected with medium or high confidence, it:

  • Applies the [Status] UI Changes label to flag the PR for documentation review
  • Sends a Slack notification to the quality channel with the AI's reasoning (currently #jetpack-developer-ambassadors in the Jetpakc repo).

Since it's a new task, it will only be enabled in the Jetpack repo for now. In the future, it can be enabled in other repos too.

Bailout conditions:

The task skips processing when:

  • No openai_api_key input is provided
  • PR is from a fork
  • PR already has [Status] UI Changes label
  • PR title contains "revert"
  • PR diff is too small to analyze

Other information:

  • Have you written new tests for your changes, if applicable?
  • Have you checked the E2E test CI results, and verified that your changes do not break them?
  • Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

N/A

Does this pull request change what data or activity we track or use?

No. The task sends PR diffs and descriptions to OpenAI for analysis but does not store or track any new data.

Testing instructions:

This is easier tested in a fork. Here are 2 examples.

check-if-docs-needed: OpenAI response for PR #213: {
  "is_user_facing": false,
  "confidence": "high",
  "reason": "The PR is a refactor that reorganizes the code related to the 5-star review link without changing its functionality or appearance to users. Users will experience the review link the same way as before, with no visible changes."
}
check-if-docs-needed: PR #213 is not user-facing or low confidence. Not adding label. Reason: The PR is a refactor that reorganizes the code related to the 5-star review link without changing its functionality or appearance to users. Users will experience the review link the same way as before, with no visible changes.

Fixes MONOREP-325

Add a new checkIfDocsNeeded task that uses OpenAI to analyze PR diffs
and descriptions to determine if changes are user-facing. If detected
with medium/high confidence, the [Status] UI Changes label is applied.

The task:
- Fetches and cleans PR diffs (filtering lock files, binary files, minified code)
- Sends to OpenAI for analysis
- Applies [Status] UI Changes label for user-facing changes
- Uses [Experiment] AI UI check marker label to prevent re-processing
- Bails out for drafts, bots, reverts, and already-labeled PRs
Copilot AI review requested due to automatic review settings January 21, 2026 11:12
@jeherve jeherve added the [Status] Needs Review This PR is ready for review. label Jan 21, 2026
@jeherve jeherve self-assigned this Jan 21, 2026
@github-actions github-actions Bot added [Action] Repo Gardening Github Action: manage PR and issues in your Open Source project Actions GitHub actions used to automate some of the work around releases and repository management Docs labels Jan 21, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖


Follow this PR Review Process:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
  3. You can use GitHub's Reviewers functionality to request a review.
  4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

@jeherve jeherve force-pushed the add/ai-ui-changes-check branch from 22f63b4 to f62b42c Compare January 21, 2026 11:16
… task

Add documentation and Slack notification support for the AI-based
user-facing changes detection task:
- Create task readme.md with usage and bailout conditions
- Update main README.md to include task in the list
- Send Slack notification to quality channel when PR is flagged
@jeherve jeherve force-pushed the add/ai-ui-changes-check branch from f62b42c to 4bab131 Compare January 21, 2026 11:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds an AI-powered task to automatically detect user-facing changes in closed PRs. The task uses OpenAI's API to analyze PR diffs and descriptions, applying a [Status] UI Changes label when user-facing changes are detected with medium or high confidence. A Slack notification is sent to the quality channel when configured.

Changes:

  • New checkIfDocsNeeded task that runs on pull_request_target closed events
  • New getDiff utility to fetch and cache PR diffs from GitHub API
  • Integration with existing OpenAI and Slack utilities for analysis and notifications

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 16 comments.

Show a summary per file
File Description
src/tasks/check-if-docs-needed/index.js Main task implementation with AI analysis logic, label application, and Slack notifications
src/tasks/check-if-docs-needed/readme.md Documentation explaining the task's purpose, behavior, and bailout conditions
src/utils/get-diff.js Utility function to fetch PR diffs with caching, filtering, and size limits
src/index.js Registers the new task to run on PR closed events with fork protection
changelog/2026-01-20-18-55-37-299081 Changelog entry documenting the new feature
README.md Updated task list to include the new checkIfDocsNeeded task

Comment thread projects/github-actions/repo-gardening/src/index.js
Comment thread projects/github-actions/repo-gardening/src/tasks/check-if-docs-needed/index.js Outdated
Comment thread projects/github-actions/repo-gardening/src/utils/get-diff.js
Comment thread projects/github-actions/repo-gardening/src/tasks/check-if-docs-needed/index.js Outdated
Comment thread projects/github-actions/repo-gardening/src/utils/get-diff.js
Comment thread projects/github-actions/repo-gardening/src/utils/get-diff.js
…cter sanitization

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Copilot AI review requested due to automatic review settings January 21, 2026 11:44
jeherve and others added 7 commits January 21, 2026 12:46
…-needed/index.js

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…-needed/index.js

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…-needed/index.js

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…-needed/index.js

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…-needed/index.js

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

Comment thread projects/github-actions/repo-gardening/src/tasks/check-if-docs-needed/index.js Outdated
Comment thread projects/github-actions/repo-gardening/src/tasks/check-if-docs-needed/readme.md Outdated
Comment thread projects/github-actions/repo-gardening/README.md
Comment thread projects/github-actions/repo-gardening/src/utils/get-diff.js
Comment thread projects/github-actions/repo-gardening/src/index.js
@jeherve jeherve requested a review from a team January 21, 2026 15:22
@jeherve jeherve added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Jan 21, 2026
@StefMattana
Copy link
Copy Markdown

@jeherve could you replace #jetpack-developer-ambassadors with #jetpack-product-ambassadors for the Slack destination ping?

Replace slack_quality_channel with slack_product_ambassadors_channel
for the check-if-docs-needed task to send notifications to the
#jetpack-product-ambassadors channel instead.
Copilot AI review requested due to automatic review settings January 21, 2026 17:53
Comment on lines +67 to +69
content = content.replace( /\\/g, '\\\\' );

return content.replace( /`/g, '\\`' );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The AI that wrote this doesn't know about using /[\\`]/g to escape both characters at once?

Also potentially of note is that a \ doesn't actually escape backticks inside backticks in (GitHub-flavored) markdown in the first place. You have to increase the number of framing backticks there too, as I did above.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done in 947ebca

Comment on lines +58 to +61
// Replace sequences of three or more backticks (which could break markdown code blocks)
// with a safe placeholder. This prevents prompt injection via code block delimiters.
// This is the critical fix: triple backticks would prematurely close the code block.
content = content.replace( /```+/g, '[code-fence]' );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In actual GitHub-flavored Markdown, you could just ensure that the real fence has more backticks than the content. For example,

This makes a code block:
```
code block!
```

I have no idea what the AI might need though.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done in 947ebca


return `You are analyzing a GitHub Pull Request to determine if users will experience something DIFFERENT after this change is deployed.

The key question is: "Will users notice any difference in behavior, appearance, or functionality?"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this the right question? We don't care about all user-facing changes, just ones that will need updates to the support documentation.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Updated in 38cc325 and 924d5dc

${ sanitizedTitle }

Here is the PR description:
${ sanitizedBody || '(No description provided)' }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You're not fencing this at all, and it might be long. How does the AI know that everything up to "Here is the code diff" isn't instruction? Particularly since it probably contains headings and all sorts of stuff.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done in 8598950

return;
}

// Skip if PR title contains "revert" (case-insensitive).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"Undo revert and fix feature with a bunch of additional UI changes"?

"New UI for post revert feature"?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Switched to a stricter approach (only look for revert at the beginning of the title) in ca0c4c1

// Check if OpenAI API key is provided.
const apiKey = getInput( 'openai_api_key' );
if ( ! apiKey ) {
debug( `check-if-docs-needed: No OpenAI key is provided. Bail.` );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems like it would be useful to output to the log even when not in testing mode. "Why did this fail? There's no diagnostic output."

Same goes for many of the error messages below.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done in f224ca5

result = JSON.parse( response );
} catch ( error ) {
debug(
`check-if-docs-needed: Failed to parse OpenAI response for PR #${ number }: ${ error }`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It may be useful to know what OpenAI did respond with, not just the parse error.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done in f224ca5

return;
}

const isUserFacing = typeof result?.is_user_facing === 'boolean' ? result.is_user_facing : false;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If it's not boolean, that probably deserves diagnostic output too.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done in f224ca5

Comment on lines +223 to +228
const confidence =
result?.confidence &&
typeof result.confidence === 'string' &&
[ 'low', 'medium', 'high' ].includes( result.confidence.trim().toLowerCase() )
? result.confidence.trim().toLowerCase()
: 'low';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same for this. Instead of (just) assuming 'low', we should probably also log the unexpected data.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done in f224ca5

// Remove unwanted file blocks (e.g., lockfiles) before further processing/truncation.
diff = filterDiff( diff );

// Filter out lines longer than 500 characters (likely minified code).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This may result in broken diffs.

At a quick grep, this will also catch a fair bit of inline SVG code. And a few paragraphs in various markdown docs.

There's also the question as to why we're committing minified code in the first place.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

58fb4b3 should solve that, although I don't know if the added complexity is worth it in this case ; I don't know that it's important for the LLM to get a valid diff since it doesn't have to apply it anywhere. I suppose it could become important in the future.

There's also the question as to why we're committing minified code in the first place.

Probably a larger discussion to have :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess my thinking was that it might confuse the AI to have broken/empty diff chunks like

diff --git a/some/file.min.js b/some/file.min.js
index aaaaaaaaaaaa..bbbbbbbbbb 100644
--- a/some/file.min.js
+++ b/some/file.min.js
@@ -1 +1
\ No newline at end of file
diff --git a/some/other.file b/some/other.file
...

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Comment thread projects/github-actions/repo-gardening/src/tasks/check-if-docs-needed/index.js Outdated
Comment thread projects/github-actions/repo-gardening/src/tasks/check-if-docs-needed/readme.md Outdated
Comment thread projects/github-actions/repo-gardening/README.md Outdated
In GitHub-flavored Markdown, backslashes don't escape backticks inside
code blocks. The proper approach is to use more backticks for the fence
than appear in the content.

- Use 4 backticks for code fences in the prompt
- Only replace sequences of 4+ backticks in content (instead of 3+)
- Remove unnecessary backslash and backtick escaping
The prompt previously asked about user-facing changes in general.
Updated to specifically ask whether support documentation would
need to be updated, which is the actual goal of this feature.
Wrap the PR title and description in code fences so the AI can
clearly distinguish between instruction text and PR content.
This prevents the AI from potentially interpreting PR content
(which may contain markdown formatting) as additional instructions.
Only skip PRs where the title starts with "Revert" (the standard
GitHub revert format), instead of skipping any PR with "revert"
anywhere in the title. This prevents incorrectly skipping PRs like
"Undo revert and fix feature" or "New UI for post revert feature".
- Include PR number in the missing API key message
- Include the actual OpenAI response when JSON parsing fails
- Log when is_user_facing is not a boolean with the actual value
- Log when confidence is not a valid value with the actual value

This makes it easier to diagnose issues when the AI returns
unexpected responses.
The previous implementation filtered all lines longer than 500 characters,
which could break diff structure by removing header lines. Now:

- Preserve diff header lines (diff --, +++, ---, @@, etc.) regardless of length
- Only filter actual content lines (additions, deletions, context)
- Add logging when lines are filtered
- Add comments explaining the tradeoff with SVG/markdown content
Update references from "quality channel/team" to "product ambassadors
channel/team" to match the actual parameter name
`slack_product_ambassadors_channel`.
Copilot AI review requested due to automatic review settings January 22, 2026 09:57
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

@jeherve
Copy link
Copy Markdown
Member Author

jeherve commented Jan 22, 2026

@jeherve could you replace #jetpack-developer-ambassadors with #jetpack-product-ambassadors for the Slack destination ping?

@StefMattana Sure thing, done!

Copy link
Copy Markdown
Contributor

@anomiex anomiex left a comment

Choose a reason for hiding this comment

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

Seems reasonable. Several comments inline, but nothing that's both too important and also is actionable IMO.

*
* In GitHub-flavored Markdown, backslashes don't escape backticks inside code blocks.
* Instead, you use more backticks for the fence than appear in the content.
* We use 4 backticks for our fences, so we replace any sequence of 4+ backticks.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Optional: We could have this function add the fencing backticks itself after scanning the content to see how many are necessary, instead of arbitrarily limiting it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh, I see what you mean. I like that. I'll keep it in mind if this becomes a problem ; I'm thinking the LLM will tolerate markup issues a bit more than a human would so this may never come up.

return `You are analyzing a GitHub Pull Request to determine if it contains changes that would require updates to user-facing support documentation.

The key question is: "Will users notice any difference in behavior, appearance, or functionality?"
The key question is: "Would support documentation need to be updated to reflect these changes?"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems like we're basically asking the AI to guess what might be on the documentation site and what might not. I wonder how well that'll work out?

This may be completely infeasible, but could we point it at the documentation we're concerned with?

Things that seem like they'd make it infeasible include if some of the documentation is private for HEs, if the AI can't access the web, or if the AI can access the web but would get bogged down trying to access the whole documentation site.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, I think it's best to keep it general for now, and we can see how accurate it is. If it makes a lot of mistakes, we could consider giving it access to the support docs to start.

debug( `check-if-docs-needed: PR #${ number } title contains "revert". Skipping.` );
// Skip if PR title starts with "Revert" (the standard GitHub revert format).
// This avoids false positives like "Undo revert and fix..." or "New UI for post revert feature".
if ( /^revert\b/i.test( title ) ) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we cover a few more cases like

Suggested change
if ( /^revert\b/i.test( title ) ) {
if ( /^revert(|ed|ing)\b/i.test( title ) ) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Looking at previous revert PRs in this repo, I think just testing for "Revert" is enough, that seems to be used every time.

// We keep diff header lines (diff --, +++, ---, @@, etc.) regardless of length to avoid
// breaking the diff format. Only actual content lines (starting with +, -, or space) are filtered.
// Note: This may also filter legitimate long lines like inline SVG or long markdown paragraphs,
// but such lines are typically not useful for AI analysis of user-facing changes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

SVGs might be relevant to evaluating whether a screenshot needs updating. Although above we only ask the AI about changed text that appears in screenshots.

Comment on lines +141 to +148
line.startsWith( 'diff --git' ) ||
line.startsWith( '---' ) ||
line.startsWith( '+++' ) ||
line.startsWith( '@@' ) ||
line.startsWith( 'index ' ) ||
line.startsWith( 'new file' ) ||
line.startsWith( 'deleted file' ) ||
line.startsWith( 'Binary files' )
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder whether this would be better as a heuristic for what to keep:

  • Line does not start with , +, -, or \ No newline.
  • Line starts with --- or +++ (or even --- a/ or +++ b/).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm thinking the existing list is maybe clearer, this way we know exactly what we keep, no exceptions.

slack_editorial_channel: ${{ vars.SLACK_EDITORIAL_CHANNEL }}
slack_he_triage_channel: ${{ vars.SLACK_HE_TRIAGE_CHANNEL }}
slack_quality_channel: ${{ vars.SLACK_QUALITY_CHANNEL }}
slack_product_ambassadors_channel: ${{ vars.SLACK_PRODUCT_AMBASSADORS_CHANNEL }}
Copy link
Copy Markdown
Contributor

@anomiex anomiex Jan 23, 2026

Choose a reason for hiding this comment

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

Thanks for documenting the new variable at PCYsg-xsv-p2 ! 🙂

@jeherve jeherve merged commit 04d65d0 into trunk Jan 29, 2026
69 checks passed
@jeherve jeherve deleted the add/ai-ui-changes-check branch January 29, 2026 14:51
@github-actions github-actions Bot added [Status] UI Changes Add this to PRs that change the UI so documentation can be updated. and removed [Status] Needs Review This PR is ready for review. labels Jan 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Action] Repo Gardening Github Action: manage PR and issues in your Open Source project Actions GitHub actions used to automate some of the work around releases and repository management Docs [Pri] Low [Status] UI Changes Add this to PRs that change the UI so documentation can be updated.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants