-
Notifications
You must be signed in to change notification settings - Fork 28
Feature/date calculation ability #116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Feature/date calculation ability #116
Conversation
- Triggers on release publish - Reuses existing build-plugin-zip workflow - Downloads built artifact - Uploads ai.zip as release asset Fixes WordPress#109
- Implements ai/calculate-dates ability as requested in WordPress#55 - Supports relative patterns (tomorrow, next Monday, in 3 days) - Supports nth weekday patterns (3rd Tuesday, first Friday, last Monday) - Supports recurring patterns (every Monday, every other Tuesday) - Supports interval patterns (every 2 weeks, every month) - Uses native PHP DateTime (no external dependencies) - Includes input validation and error handling - REST API enabled - Limits occurrences to 52 maximum - Requires edit_posts capability Addresses WordPress#55
|
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @alishanvr, @jmarx. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #116 +/- ##
=============================================
+ Coverage 29.37% 32.56% +3.18%
- Complexity 145 215 +70
=============================================
Files 14 17 +3
Lines 885 1207 +322
=============================================
+ Hits 260 393 +133
- Misses 625 814 +189
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| * @return bool True if pattern is relative. | ||
| */ | ||
| private function is_relative_pattern( string $pattern ): bool { | ||
| return (bool) preg_match( '/^(tomorrow|yesterday|today|next\s+\w+day|in\s+\d+\s+(day|week|month|year)s?)$/i', $pattern ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious how this, and related methods, might work with non-English languages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should pattern keywords also be translatable (e.g., users can type "demain" instead of "tomorrow"), or just the UI strings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, don't think this type of pattern matching will work for any languages outside of english. Ideally we find a better way to do this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't see your comment while I was leaving mine but ideally we find an approach that will work irregardless of the language. I'm not sure off the top of my head the best way to do that but having hardcoded strings here will prevent that from working
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After thinking about this more, what if we move away from natural language parsing entirely and use a structured input format instead?
Current (language-dependent):
{
"pattern": "3rd Tuesday"
}
Proposed (language-neutral):
{
"type": "nth_weekday",
"weekday": 2, // 0=Sunday, 1=Monday, 2=Tuesday...
"nth": 3
}
===
Some Examples:
Every Monday:
{
"type": "recurring",
"weekday": 1,
"interval": 1
}
First Friday
{
"type": "nth_weekday",
"weekday": 5,
"nth": 1
}
Every 2 weeks
{
"type": "interval",
"unit": "week",
"amount": 2
}
What do you think?
.github/workflows/release.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming this file wasn't meant to be part of this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's related to #109
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally this PR stays focused on just the needed changes and doesn't include code from #109
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /** | ||
| * Date calculation utility WordPress Abilities. | ||
| * | ||
| * @since 0.2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can update all @since statements to x.x.x and these will be updated during release
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I will use x.x.x
I will update it by the end of this week.
is this ok? Or you need it more urgent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is fine, no rush on my end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in cf1c7c0
| 'execute_callback' => array( $this, 'execute_calculate_dates' ), | ||
| 'permission_callback' => array( $this, 'permission_callback' ), | ||
| 'meta' => array( | ||
| 'show_in_rest' => true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be surfaced in the REST API? Seems like surfacing this via MCP would suffice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not sure. So, I added to show in rest as well.
If we don't need to show in rest then I will remove it. Please confirm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now I would leave this argument off
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the show_in_rest in 5f7681b
| */ | ||
| public function execute_calculate_dates( array $input ) { | ||
| // Validate pattern is provided. | ||
| if ( empty( $input['pattern'] ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is set as required in our schema, no need for the extra check here I don't think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, i will fix this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 78a870a
| */ | ||
| public function permission_callback( array $args ) { | ||
| // Anyone who can edit posts can use date calculations. | ||
| if ( ! current_user_can( 'edit_posts' ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the thought process behind this permission check? It doesn't seem like we're interacting with any WordPress data so could argue no need for a capability check and maybe fine with just a user logged in check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! My thinking was to require some minimum capability to prevent anonymous users from making potentially resource-intensive date calculations.
However, I agree - since we're not reading/writing WordPress data, just doing date math, a logged-in check would be sufficient.
Should I change it to:
public function permission_callback( array $args ) {
return is_user_logged_in();
}Or remove the permission check entirely and allow public access?
Let me know your preference!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could see a case to make for keeping this open to everyone but for now, probably safer to have the is_user_logged_in check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed/Updated in 8f2687a
…set as required already in base
What?
Adds
ai/calculate-datesability to calculate dates from natural language patterns.Closes #55
This PR implements a date calculation ability that helps AI assistants accurately calculate recurring dates from natural language patterns like "3rd Tuesday of each month" or "every other Friday".
Why?
As noted in #55, AI models struggle with date calculations, especially recurring patterns. This ability provides:
How?
Implementation:
Date_Calculationclass inincludes/Abilities/Utilities/ai/calculate-datesabilityshow_in_rest => truePostsclass)Architecture:
DateTimeclass (no external dependencies)edit_postscapabilitySupported Patterns
tomorrow,next Monday,in 3 days3rd Tuesday,first Friday,last Mondayevery Monday,every other Tuesdayevery 2 weeks,every month,every 3 daysTesting Instructions
Via WP-CLI
Expected Results
Input:
{ "pattern": "every monday", "occurrences": 3 }Output:
{ "dates": [ "2025-12-01T00:00:00+00:00", "2025-12-08T00:00:00+00:00", "2025-12-15T00:00:00+00:00" ], "pattern": "every monday" }OR
wp eval "$ability = wp_get_ability('ai/calculate-dates'); print_r($ability->execute(array('pattern' => 'every monday', 'occurrences' => 5)));"
Testing Instructions for Keyboard
N/A - This PR does not affect the user interface. It only adds backend ability for date calculations.
Screenshots or screencast
N/A - Backend ability with no visual output. The ability is accessible via PHP, WP-CLI, and REST API.
Testing Checklist
Code Quality
Notes
Postsabilitybootstrap.phpalongside other utilities