-
Notifications
You must be signed in to change notification settings - Fork 33
Allow optimal performance_blending_weight auto-selection #3694
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
Allow optimal performance_blending_weight auto-selection #3694
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (2)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the 📝 WalkthroughWalkthroughRemoved the performance blending weight parameter across Traffic Boost layers: provider options and payloads, REST API routes and handlers, validations, and Suggestions API endpoint types/payloads. Deleted the validator class. Requests now omit performance_blending_weight and rely on server defaults. No other control flow changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/services/suggestions-api/endpoints/class-endpoint-suggest-inbound-links.php (1)
63-64: Clamp and cast max_items before sending to the API.Defensively ensure an integer ≥ 1 to avoid surprising upstream behavior.
- 'max_items' => $options['max_items'] ?? 10, + 'max_items' => max( 1, (int) ( $options['max_items'] ?? 10 ) ),src/rest-api/content-helper/class-endpoint-traffic-boost.php (4)
93-119: Add min/max bounds tomax_itemsREST schema.Declare limits in the route args so invalid values are rejected early and payload sizes are bounded.
'max_items' => array( 'type' => 'integer', 'description' => __( 'The maximum number of suggestions to return.', 'wp-parsely' ), 'default' => 10, + 'minimum' => 1, + 'maximum' => 50, 'required' => false, ),
131-159: Validate/sanitize string lists for placement routes.
ignore_keywordsandkeyword_exclusion_listaccept unvalidated arrays. Add callbacks to normalize to arrays of strings.'ignore_keywords' => array( 'type' => 'array', 'description' => __( 'The keywords to ignore.', 'wp-parsely' ), 'required' => false, + 'sanitize_callback' => array( __CLASS__, 'sanitize_string_array' ), + 'validate_callback' => array( __CLASS__, 'validate_string_array' ), ), 'keyword_exclusion_list' => array( 'type' => 'array', 'description' => __( 'The keywords to exclude from the suggestions.', 'wp-parsely' ), 'required' => false, 'default' => array(), + 'sanitize_callback' => array( __CLASS__, 'sanitize_string_array' ), + 'validate_callback' => array( __CLASS__, 'validate_string_array' ), ),Add these helpers in this class (outside the shown hunk):
private static function sanitize_string_array( $value ): array { $arr = is_array( $value ) ? $value : array(); return array_values( array_filter( array_map( 'sanitize_text_field', $arr ), 'strlen' ) ); } private static function validate_string_array( $value ) { if ( null === $value ) { return true; } if ( ! is_array( $value ) ) { return new \WP_Error( 'parsely_invalid_param', __( 'Expected an array of strings.', 'wp-parsely' ) ); } foreach ( $value as $v ) { if ( ! is_string( $v ) ) { return new \WP_Error( 'parsely_invalid_param', __( 'Expected an array of strings.', 'wp-parsely' ) ); } } return true; }
346-348: Normalize request options sent to Suggestions API.Cast and de-duplicate inputs to reduce surprises downstream.
- 'max_items' => $max_items, - 'url_exclusion_list' => $url_exclusion_list, + 'max_items' => max( 1, (int) $max_items ), + 'url_exclusion_list' => array_values( array_unique( (array) $url_exclusion_list ) ),
454-455: Sanitize and de-duplicatekeyword_exclusion_listbefore forwarding.Pre-cleaning helps the Suggestions API and avoids false in_array matches later.
- 'keyword_exclusion_list' => $keyword_exclusion_list, + 'keyword_exclusion_list' => array_values( + array_unique( array_map( 'sanitize_text_field', (array) $keyword_exclusion_list ) ) + ),src/content-helper/dashboard-page/pages/traffic-boost/provider.ts (2)
293-299: Document the removal in JSDoc with a new @SInCE entry.Add a second @SInCE line noting the auto-selection change so downstream integrators notice the behavior shift.
/** * Generates suggestions for a given post. * - * @since 3.19.0 + * @since 3.19.0 + * @since X.Y.Z Removed performanceBlendingWeight; server now auto-selects the optimal blending weight. * * @param {number} postId The ID of the post to generate suggestions for. * @param {Object} options The options for the suggestions. * @param {number} options.maxItems The maximum number of items to generate. * @param {boolean} options.discardPrevious Whether to discard previous suggestions. * @param {string[]} options.urlExclusionList The list of URLs to exclude from the suggestions. * @param {boolean} options.save Whether to save the suggestions.Please confirm the next plugin version to replace X.Y.Z.
333-340: Mirror the JSDoc change for placement generation.Reflect the blending-weight removal here too for consistency.
/** * Generates a placement suggestion for a given post. * - * @since 3.19.0 + * @since 3.19.0 + * @since X.Y.Z Removed performanceBlendingWeight; server now auto-selects the optimal blending weight. * * @param {HydratedPost} sourcePost The source post. * @param {HydratedPost} destinationPost The destination post. * @param {TrafficBoostLink} trafficBoostLink The traffic boost link to generate a placement for. * @param {Object} options The options for the suggestion. * @param {string[]} options.ignoreKeywords The keywords to ignore. * @param {boolean} options.save Whether to save the suggestion. * @param {boolean} options.allowDuplicateLinks Whether to allow duplicate links.Also confirm X.Y.Z.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
build/content-helper/dashboard-page.asset.phpis excluded by!build/**build/content-helper/dashboard-page.jsis excluded by!build/**
📒 Files selected for processing (7)
src/content-helper/dashboard-page/pages/traffic-boost/provider.ts(2 hunks)src/rest-api/content-helper/class-endpoint-traffic-boost.php(4 hunks)src/rest-api/content-helper/validations/class-validate-blending-weight.php(0 hunks)src/services/suggestions-api/endpoints/class-endpoint-suggest-headline.php(0 hunks)src/services/suggestions-api/endpoints/class-endpoint-suggest-inbound-link-positions.php(0 hunks)src/services/suggestions-api/endpoints/class-endpoint-suggest-inbound-links.php(1 hunks)src/services/suggestions-api/endpoints/class-endpoint-suggest-linked-reference.php(0 hunks)
💤 Files with no reviewable changes (4)
- src/rest-api/content-helper/validations/class-validate-blending-weight.php
- src/services/suggestions-api/endpoints/class-endpoint-suggest-headline.php
- src/services/suggestions-api/endpoints/class-endpoint-suggest-inbound-link-positions.php
- src/services/suggestions-api/endpoints/class-endpoint-suggest-linked-reference.php
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{html,php}
⚙️ CodeRabbit configuration file
**/*.{html,php}: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the HTML and PHP code to ensure it is well-structured and adheres to best practices.
- Ensure the code follows WordPress coding standards and is well-documented.
- Confirm the code is secure and free from vulnerabilities.
- Optimize the code for performance, removing any unnecessary elements.
- Validate comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Verify code compatibility with the latest version of WordPress, avoiding deprecated functions or features."
Files:
src/rest-api/content-helper/class-endpoint-traffic-boost.phpsrc/services/suggestions-api/endpoints/class-endpoint-suggest-inbound-links.php
**/*.{js,ts,tsx,jsx}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,jsx}: "Perform a detailed review of the provided code with following key aspects in mind:
- Review the code to ensure it is well-structured and adheres to best practices.
- Verify compliance with WordPress coding standards.
- Ensure the code is well-documented.
- Check for security vulnerabilities and confirm the code is secure.
- Optimize the code for performance, removing any unnecessary elements.
- Validate JSDoc comments for accuracy, currency, and adherence to WordPress coding standards.
- Ensure each line comment concludes with a period.
- Confirm every JSDoc comment includes a @SInCE tag indicating the next version of the plugin to include the code.
- Guarantee compatibility with the latest version of WordPress, avoiding deprecated functions or features."
Files:
src/content-helper/dashboard-page/pages/traffic-boost/provider.ts
🔇 Additional comments (1)
src/rest-api/content-helper/class-endpoint-traffic-boost.php (1)
93-119: No remaining references to performance_blending_weight found.Scanned repository with rg/git-grep/grep and inspected src/rest-api/content-helper/class-endpoint-traffic-boost.php (lines 1–700); no matches.
…ight-auto-selection
…mance-blending-weight-auto-selection" (7177001)
Description
The
performance_blending_weightAPI parameter was hardcoded into plugin code. With this PR we're removing the related code, so the optimalperformance_blending_weightvalue can be auto-selected by the API.A possible alternative was to set the default value to
null, and not send the parameter to the API whennull. But removing was simpler and we don't currently have any UI for this code. It will be easy to restore this if we ever need it.Closes #3696.
Summary by CodeRabbit