[WIP] #48: Create AI branch review command#49
Conversation
- Add `ddev review` command wrapper. - Implement core `review.sh` script for AI-powered code reviews. - Compare current branch against a target branch (default: main) using git diff. - Integrate with OpenAI-compatible APIs for review generation. - Handle API key validation, diff truncation, and detailed prompt construction. - Provide error handling for API calls and response parsing. - Support `glow` for enhanced Markdown rendering of the review report.
5768cbb to
9ba66ae
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new DDEV command to run an AI-assisted code review of the current Git branch against a target branch (defaulting to main).
Changes:
- Introduces
wunderio/core/tooling/review.shto generate a diff and request an AI review via the configured OpenAI-compatible API. - Adds a
ddev reviewcommand wrapper to invoke the tooling script viawdr-core.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| wunderio/core/tooling/review.sh | Implements the branch diff gathering, request payload building, API call, and rendering of the AI review output. |
| commands/web/wunderio-core-review.sh | Registers the ddev review command and routes it to the core tooling script. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| TARGET_BRANCH="${1:-main}" | ||
| CURRENT_BRANCH=$(git branch --show-current 2>/dev/null || echo "HEAD") | ||
|
|
||
| if [ "$CURRENT_BRANCH" = "$TARGET_BRANCH" ]; then | ||
| echo "ℹ️ You are currently on the '$TARGET_BRANCH' branch. Switch to a feature branch to perform a review." | ||
| exit 0 |
There was a problem hiding this comment.
git branch --show-current returns an empty string (exit 0) in detached HEAD state, so the || echo "HEAD" fallback won’t run and CURRENT_BRANCH becomes empty. Handle the empty-string case explicitly so messages and comparisons don’t break (e.g., default to "HEAD" when the output is empty).
| HTTP_STATUS=$(cat "$CURL_HTTP_CODE_FILE" 2>/dev/null || echo "") | ||
| RESPONSE=$(cat "$CURL_RESPONSE_FILE" 2>/dev/null || echo "") | ||
|
|
||
| # Handle curl errors | ||
| if [ $CURL_EXIT_CODE -ne 0 ] || [ "$HTTP_STATUS" -lt 200 ] ||[ "$HTTP_STATUS" -ge 300 ]; then | ||
| display_error_message "❌ Error: API request failed (cURL code: $CURL_EXIT_CODE, HTTP Status: $HTTP_STATUS)" | ||
| if [ -n "$RESPONSE" ]; then | ||
| echo "API Response (truncated): ${RESPONSE:0:300}..." | ||
| fi | ||
| exit 1 |
There was a problem hiding this comment.
HTTP status handling assumes $HTTP_STATUS is a valid number. If it’s empty or non-numeric (e.g., curl didn’t write the file), [ "$HTTP_STATUS" -lt 200 ] / -ge 300 will emit errors and may skip the intended failure path. Consider validating that the status is numeric (or explicitly treating empty/"000" as an error) before doing numeric comparisons, similar to the more defensive logic in tooling/commit.sh.
| # Usage: ddev ai-review [target-branch] | ||
| # Example: ddev ai-review | ||
| # Example: ddev ai-review develop |
There was a problem hiding this comment.
The header comment documents ddev ai-review, but this command is wired up as ddev review (see commands/web/wunderio-core-review.sh). Update the usage/examples in this script header to match the actual command name to avoid confusing users.
| # Usage: ddev ai-review [target-branch] | |
| # Example: ddev ai-review | |
| # Example: ddev ai-review develop | |
| # Usage: ddev review [target-branch] | |
| # Example: ddev review | |
| # Example: ddev review develop |
| source "$WUNDERIO_GLOBAL_SCRIPT_ROOT/_helpers.sh" 2>/dev/null || true | ||
|
|
||
| # Helper function for error messages if _helpers.sh isn't available | ||
| display_error_message() { | ||
| echo -e "\033[31m$1\033[0m" | ||
| } |
There was a problem hiding this comment.
With set -u, source "$WUNDERIO_GLOBAL_SCRIPT_ROOT/_helpers.sh" will still error if WUNDERIO_GLOBAL_SCRIPT_ROOT is unset (even though stderr is redirected). If you want this to be optional, guard the reference with ${WUNDERIO_GLOBAL_SCRIPT_ROOT:-} (and a file check). Also, this unconditional display_error_message() definition will override the helper’s implementation when helpers are sourced; consider only defining the fallback if the function is not already declared.
| source "$WUNDERIO_GLOBAL_SCRIPT_ROOT/_helpers.sh" 2>/dev/null || true | |
| # Helper function for error messages if _helpers.sh isn't available | |
| display_error_message() { | |
| echo -e "\033[31m$1\033[0m" | |
| } | |
| if [[ -n "${WUNDERIO_GLOBAL_SCRIPT_ROOT:-}" ]] && [[ -f "${WUNDERIO_GLOBAL_SCRIPT_ROOT}/_helpers.sh" ]]; then | |
| # shellcheck disable=SC1090 | |
| source "${WUNDERIO_GLOBAL_SCRIPT_ROOT}/_helpers.sh" | |
| fi | |
| # Helper function for error messages if _helpers.sh isn't available | |
| if ! declare -F display_error_message >/dev/null 2>&1; then | |
| display_error_message() { | |
| echo -e "\033[31m$1\033[0m" | |
| } | |
| fi |
Overview
Screenshots
Testing
git clone -b feature/#48-Create-AI-branch-review-command git@github.com:wunderio/ddev-wunderio-drupal.git && ddev add-on get ./ddev-wunderio-drupal && ddev restart