-
-
Notifications
You must be signed in to change notification settings - Fork 29
Add script to trigger AI evaluation #607
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: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds a new maintenance script RequeueForAIEvaluation that queries cw_requests from a given timestamp for a target queue (default 'inreview') and requeues matching requests as RequestWikiRemoteAIJob jobs into JobQueueGroup, with optional sleep between enqueues and configurable queue name. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin as Admin/CLI
participant Script as RequeueForAIEvaluation
participant DB as Primary DB
participant Queue as JobQueueGroup
Admin->>Script: Run with --from, [--queue], [--sleep]
Script->>DB: SELECT id FROM cw_requests WHERE cw_status = queue AND cw_timestamp > from
alt No rows
Script-->>Admin: "No matching rows"
else Rows found
loop For each request
Script->>Queue: push Job(RequestWikiRemoteAIJob, requestId)
Note right of Script: Sleep N seconds (default 30)
end
Script-->>Admin: "Completed requeue"
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 1
🧹 Nitpick comments (3)
maintenance/RequeueForAIEvaluation.php (3)
79-81: Cast sleep option to integer.The
getOptionmethod returns a string, butsleep()expects an integer. While PHP will coerce the type, explicit casting improves type safety and clarity.Apply this diff:
- $sleepFor = $this->getOption( 'sleep', 30 ); + $sleepFor = (int)$this->getOption( 'sleep', 30 ); $this->output( "Sleeping for $sleepFor seconds...\n" ); sleep( $sleepFor );
50-58: Consider adding timestamp validation.The user-supplied
fromargument is passed directly to$dbw->timestamp()without validation. While MediaWiki's timestamp method handles various formats, explicit validation would improve error messages and prevent unexpected behavior.Consider adding validation:
$from = $this->getArg( 'from' ); // Validate timestamp format (should be 14 digits: YYYYMMDDHHmmss) if ( !preg_match( '/^\d{14}$/', $from ) ) { $this->fatalError( "Invalid timestamp format. Expected YYYYMMDDHHmmss, got: $from" ); } $res = $dbw->newSelectQueryBuilder() // ... rest of query
67-82: Consider adding operational safeguards.For large result sets, this script could run for extended periods. Consider adding:
- A
--limitoption to process only N requests per run- A
--dry-runmode to preview without queuing jobs- Progress indicators (e.g., "Processing 50/1000...")
- Logging request IDs to a file for audit trails
Example additions to constructor:
$this->addOption( 'limit', 'Maximum number of requests to process', required: false, withArg: true ); $this->addOption( 'dry-run', 'Preview requests without queuing jobs', required: false );Example in execute:
$limit = $this->getOption( 'limit' ); $dryRun = $this->hasOption( 'dry-run' ); $count = 0; foreach ( $res as $row ) { if ( $limit && $count >= $limit ) { $this->output( "Reached limit of $limit requests.\n" ); break; } $requestId = (int)$row->cw_id; $this->output( "Processing request $requestId...\n" ); if ( !$dryRun ) { // ... queue job } else { $this->output( "[DRY RUN] Would queue request $requestId\n" ); } $count++; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
maintenance/RequeueForAIEvaluation.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
⚙️ CodeRabbit configuration file
**/*.php: Follow MediaWiki PHP Coding Standards:Indentation:
- Use tabs for indentation, not spaces.
Spaces:
- Use spaces on either side of binary operators.
Example: $a = $b + $c; (Correct) $a=$b+$c; (Incorrect)- Add spaces inside parentheses, except where they are empty.
Example: $a = getFoo( $b ); (Correct) $a = getFoo($b); (Incorrect)- Do not add a space after a function name.
Example: $c = getBar(); (Correct) $c = getBar (); (Incorrect)Function Return Type Hint:
- Add a space after the colon in the function return type hint.
Example: function square( int $x ): int { return $x * $x; }Arrays:
- Use spaces in brackets when declaring an array, except where the array is empty.
Example: $a = [ 'foo', 'bar' ]; (Correct) $a = ['foo', 'bar']; (Incorrect)- Do not put spaces in brackets when accessing array elements.
Example: $c = $a[0]; (Correct) $c = $a[ 0 ]; (Incorrect)Control Structures:
- Add a space after control structure keywords.
Example: if ( isFoo() ) { ... } (Correct) if(isFoo()){ ... } (Incorrect)Type Casting:
- Do not use a space within or after the cast operator.
Example: (int)$foo; (Correct) (int) $bar; (Incorrect)Comments:
- In comments, there should be one space between the # or // and the comment text.
Example: // This is a comment. (Correct) //This is a comment. (Incorrect)Ternary Operator:
- Keep ternary operator expressions short and obvious.
- For multi-line ternary expressions, place the '?' and ':' at the beginning of the new lines.
File Formatting:
- Avoid trailing whitespace on lines.
Files:
maintenance/RequeueForAIEvaluation.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: call-mediawiki-tests-workflow (master, 8.4, 84, false, false, true, true) / master | PHP 8.4
- GitHub Check: call-mediawiki-tests-workflow (REL1_44, 8.2, 82, false, true, false, false) / REL1_44 | PHP 8.2
- GitHub Check: call-mediawiki-tests-workflow (REL1_44, 8.4, 84, true, false, false, true) / REL1_44 | PHP 8.4
- GitHub Check: call-mediawiki-tests-workflow (master, 8.2, 82, false, false, true, true) / master | PHP 8.2
🔇 Additional comments (4)
maintenance/RequeueForAIEvaluation.php (4)
1-30: LGTM!The GPL license header, namespace declaration, and use statements are correct and follow MediaWiki conventions.
88-90: LGTM!The return statement with code coverage markers follows standard MediaWiki maintenance script conventions.
33-40: Fix spacing to comply with MediaWiki PHP coding standards.Multiple lines violate the MediaWiki requirement to use spaces inside parentheses for function calls.
As per coding guidelines, apply this diff:
public function __construct() { parent::__construct(); - $this->addDescription( "Loads wiki requests in a defined queue (defaults to 'inreview') for AI evaluation." ); + $this->addDescription( 'Loads wiki requests in a defined queue (defaults to \'inreview\') for AI evaluation.' ); - $this->addArg( 'from', 'Timestamp defining from when to start loading requests.', required: true ); - $this->addOption( 'sleep', 'How many seconds the script should sleep for', required: false, withArg: true ); - $this->addOption( 'queue-name', 'What queue should be processed?', required: false, withArg: true ); + $this->addArg( 'from', 'Timestamp defining from when to start loading requests.', required: true ); + $this->addOption( 'sleep', 'How many seconds the script should sleep for', required: false, withArg: true ); + $this->addOption( 'queue-name', 'What queue should be processed?', required: false, withArg: true ); }Likely an incorrect or invalid review comment.
42-85: Fix spacing throughout execute method to comply with MediaWiki PHP coding standards.Multiple function calls are missing required spaces inside parentheses per MediaWiki coding standards.
As per coding guidelines, apply this diff:
public function execute() { - $dbw = MediaWikiServices::getInstance() - ->getDBLoadBalancer() - ->getConnection( DB_PRIMARY, 'virtual-createwiki-central' ); + $dbw = MediaWikiServices::getInstance() + ->getDBLoadBalancer() + ->getConnection( DB_PRIMARY, 'virtual-createwiki-central' ); $queueName = $this->getOption( 'queue-name', 'inreview' ); $this->output( "Fetching all '$queueName' requests...\n" ); - $from = $this->getArg( 'from', '20250901000000' ); + $from = $this->getArg( 'from' ); $res = $dbw->newSelectQueryBuilder() ->select( 'cw_id' ) ->from( 'cw_requests' ) ->where( [ 'cw_status' => $queueName ] ) ->andWhere( $dbw->expr( 'cw_timestamp', '>', $dbw->timestamp( $from ) ) ) ->caller( __METHOD__ ) ->fetchResultSet(); if ( !$res->numRows() ) { $this->output( "No requests found with status '$queueName' within the specified range.\n" ); return; } $jobQueueGroup = MediaWikiServices::getInstance()->getJobQueueGroup(); foreach ( $res as $row ) { - $requestId = (int)$row->cw_id; + $requestId = (int)$row->cw_id; $this->output( "Adding wiki request $requestId to queue...\n" ); $job = new JobSpecification( 'RequestWikiRemoteAIJob', [ 'id' => $requestId ] ); $jobQueueGroup->push( $job ); $this->output( "Successfully added wiki request $requestId to the AI evaluation queue!\n" ); - $sleepFor = $this->getOption( 'sleep', 30 ); + $sleepFor = (int)$this->getOption( 'sleep', 30 ); $this->output( "Sleeping for $sleepFor seconds...\n" ); - sleep( $sleepFor ); + sleep( $sleepFor ); } $this->output( "All '$queueName' requests have been queued for processing.\n" ); }Likely an incorrect or invalid review comment.
| $queueName = $this->getOption( 'queue-name', 'inreview' ); | ||
| $this->output( "Fetching all '$queueName' requests...\n" ); | ||
|
|
||
| $from = $this->getArg( 'from', '20250901000000' ); |
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.
Remove the default value from the required argument.
Line 37 declares the from argument as required: true, but Line 50 provides a default value '20250901000000'. This makes the argument effectively optional. Remove the default value to enforce the requirement, or make the argument optional in the constructor.
Apply this diff:
- $from = $this->getArg( 'from', '20250901000000' );
+ $from = $this->getArg( 'from' );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $from = $this->getArg( 'from', '20250901000000' ); | |
| $from = $this->getArg( 'from' ); |
🤖 Prompt for AI Agents
In maintenance/RequeueForAIEvaluation.php around line 50, the call to
$this->getArg provides a default ('20250901000000') even though the constructor
declares the 'from' argument required; remove the default so the call enforces
the required argument (i.e., call getArg with just the argument name and no
default), or alternatively change the constructor to make 'from' optional if you
intend to keep the default—update the code accordingly so the requirement and
default behavior are consistent.
No description provided.