bito-code-review[bot] commented on code in PR #40823:
URL: https://github.com/apache/superset/pull/40823#discussion_r3366637151
##########
tests/unit_tests/sql_lab_execution_context.py:
##########
@@ -101,3 +104,32 @@ def test_create_table_as_select():
assert ctas.ctas_method == CTASMethod.TABLE
assert ctas.target_schema_name == "public"
assert ctas.target_table_name == "temp_table"
+
+
+def test_set_database_accepts_matching_id(query_params):
+ context = SqlJsonExecutionContext(query_params)
+ database = MagicMock()
+ database.id = 1
+ database.get_default_catalog.return_value = "default_catalog"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Dead mock setup in test</b></div>
<div id="fix">
The mock setup on line 113 (`database.get_default_catalog.return_value`) is
dead code. The `query_params` fixture provides `catalog: "default"`, so when
`set_database` executes line 129's condition `if self.catalog is None:`, it
evaluates to `False` — the method never calls `database.get_default_catalog()`.
Either remove this unused mock setup, or add an assertion verifying the mock
was called.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- a/tests/unit_tests/sql_lab_execution_context.py
+++ b/tests/unit_tests/sql_lab_execution_context.py
@@ -110,7 +110,6 @@ def test_set_database_accepts_matching_id(query_params):
context = SqlJsonExecutionContext(query_params)
database = MagicMock()
database.id = 1
- database.get_default_catalog.return_value = "default_catalog"
context.set_database(database)
```
</div>
</details>
</div>
<small><i>Code Review Run #6cef4a</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
scripts/devin_autofix/start_session.py:
##########
@@ -0,0 +1,303 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Start a Devin autofix session for a GitHub issue.
+
+Usage:
+ python -m scripts.devin_autofix.start_session \
+ --issue-number 123 \
+ --trigger-comment-id 456789 \
+ --triggered-by vickyli1014 \
+ --run-url "https://github.com/.../actions/runs/999";
+"""
+
+from __future__ import annotations
+
+import argparse
+import json
+import os
+import sys
+from datetime import datetime, timezone
+
+import requests
+
+from scripts.devin_autofix.github_state import (
+ add_label,
+ classify_issue_type,
+ detect_acceptance_criteria,
+ find_active_markers,
+ get_issue,
+ post_comment,
+ remove_label,
+ REPO,
+ RunMarker,
+)
+
+DEVIN_API = "https://api.devin.ai/v3";
+
+STRUCTURED_OUTPUT_SCHEMA = {
+ "type": "object",
+ "properties": {
+ "issue_number": {"type": "integer"},
+ "issue_type": {"type": "string"},
+ "summary": {"type": "string"},
+ "status": {
+ "type": "string",
+ "enum": [
+ "pr_opened",
+ "waiting_review",
+ "returned_to_human",
+ "failed",
+ ],
+ },
+ "acceptance_criteria_source": {
+ "type": "string",
+ "enum": ["issue", "ai_inferred", "none"],
+ },
+ "reproduction_result": {
+ "type": "string",
+ "enum": [
+ "reproduced",
+ "validated_by_code_inspection",
+ "not_reproduced",
+ "not_attempted",
+ ],
+ },
+ "test_commit": {"type": ["string", "null"]},
+ "implementation_commit": {"type": ["string", "null"]},
+ "pr_url": {"type": ["string", "null"]},
+ "checks_run": {
+ "type": "array",
+ "items": {
+ "type": "object",
+ "properties": {
+ "name": {"type": "string"},
+ "conclusion": {"type": "string"},
+ },
+ "required": ["name", "conclusion"],
+ },
+ },
+ "return_reason": {"type": ["string", "null"]},
+ "docs_updated": {"type": "boolean"},
+ },
+ "required": [
+ "issue_number",
+ "issue_type",
+ "summary",
+ "status",
+ "acceptance_criteria_source",
+ "reproduction_result",
+ "pr_url",
+ "return_reason",
+ "docs_updated",
+ ],
+}
+
+
+def build_prompt(issue_number: int, issue_url: str, issue_body: str | None) ->
str:
+ """Build the Devin session prompt."""
+ return f"""You are resolving GitHub issue {issue_url} in repo {REPO}.
+
+Rules:
+- Diagnose the issue before editing.
+- Reproduce or validate the issue when practical.
+- If acceptance criteria are present in the issue, use them.
+- If not present, infer minimal acceptance criteria and mark them as
AI-inferred.
+- Create focused failing tests first.
+- Commit test-only changes.
+- Make an implementation plan.
+- Implement the plan without modifying the tests committed in the test-only
commit.
+- Commit implementation changes separately.
+- Run focused validation.
+- Open a PR using the repo PR template.
+- Link the PR to the issue using Fixes #{issue_number}.
+- Do not request reviewers yourself; the orchestrator will request review.
+- Do not comment the PR link on the issue; the monitor workflow owns updates.
+- Update docs only if the change affects user-facing behavior.
+- Return structured JSON matching the requested schema.
+
+Return to human if:
+- The issue is security-sensitive.
+- The issue cannot be reproduced or validated enough to design a test.
+- Required secrets or external systems are unavailable.
+- The fix requires a DB migration or large architectural change.
+- Focused validation remains failing after reasonable attempts.
+"""
+
+
+def create_devin_session(
+ issue_number: int,
+ issue_url: str,
+ issue_body: str | None,
+ run_id: str,
+) -> dict[str, str]:
+ """Call the Devin API to create an autofix session.
+
+ Returns dict with ``session_id`` and ``url``.
+ """
+ api_key = os.environ["DEVIN_API_KEY"]
+ org_id = os.environ["DEVIN_ORG_ID"]
+
+ prompt = build_prompt(issue_number, issue_url, issue_body)
+
+ payload = {
+ "prompt": prompt,
+ "title": f"Autofix issue #{issue_number}",
+ "repos": [REPO],
+ "tags": [
+ "autofix",
+ f"repo:{REPO}",
+ f"issue:{issue_number}",
+ f"run:{run_id}",
+ ],
+ "structured_output_schema": STRUCTURED_OUTPUT_SCHEMA,
+ }
+
+ resp = requests.post(
+ f"{DEVIN_API}/organizations/{org_id}/sessions",
+ headers={
+ "Authorization": f"Bearer {api_key}",
+ "Content-Type": "application/json",
+ },
+ json=payload,
+ timeout=60,
+ )
+ resp.raise_for_status()
+ data = resp.json()
+ return {
+ "session_id": data["session_id"],
+ "url": data.get("url",
f"https://app.devin.ai/sessions/{data['session_id']}"),
+ }
+
+
+def main() -> None:
+ parser = argparse.ArgumentParser(description="Start a Devin autofix
session")
+ parser.add_argument("--issue-number", type=int, required=True)
+ parser.add_argument("--trigger-comment-id", type=int, required=True)
+ parser.add_argument("--triggered-by", required=True)
+ parser.add_argument("--run-url", required=True)
+ args = parser.parse_args()
+
+ issue_number: int = args.issue_number
+ trigger_comment_id: int = args.trigger_comment_id
+ triggered_by: str = args.triggered_by
+ run_url: str = args.run_url
+
+ # Validate issue is open
+ issue = get_issue(issue_number)
+ if issue.get("state") != "open":
+ print(f"::error::Issue #{issue_number} is not open")
+ sys.exit(1)
+
+ # Check for duplicate active runs
+ if active := find_active_markers(issue_number):
+ post_comment(
+ issue_number,
+ f"⚠️ Cannot start a new autofix run — there is already an active
run "
+ f"(`{active[0].run_id}`) on this issue. Wait for it to complete or
be "
+ f"returned to human before retrying.",
+ )
+ print(f"::error::Duplicate active run found: {active[0].run_id}")
+ sys.exit(1)
+
+ # Classify issue
+ issue_type = classify_issue_type(issue)
+ ac_source = detect_acceptance_criteria(issue.get("body"))
+ now = datetime.now(timezone.utc)
+ run_id = f"issue-{issue_number}-{now.strftime('%Y-%m-%dT%H:%M:%SZ')}"
+
+ # Post initial comment
+ marker = RunMarker(
+ run_id=run_id,
+ issue_number=issue_number,
+ trigger_comment_id=trigger_comment_id,
+ triggered_by=triggered_by,
+ status="in_progress",
+ acceptance_criteria_source=ac_source,
+ created_at=now.isoformat(),
+ updated_at=now.isoformat(),
+ )
+
+ initial_body = (
+ f"🤖 **Devin Autofix** — run started\n\n"
+ f"- **Issue type:** {issue_type}\n"
+ f"- **Acceptance criteria:** {ac_source}\n"
+ f"- **Triggered by:** @{triggered_by}\n"
+ f"- **Workflow run:** [link]({run_url})\n\n"
+ f"Devin is working on this issue. A PR will be opened when ready.\n\n"
+ f"{marker.to_marker()}"
+ )
+ post_comment(issue_number, initial_body)
+
+ # Start Devin session
+ try:
+ session = create_devin_session(
+ issue_number,
+ issue["html_url"],
+ issue.get("body"),
+ run_id,
+ )
+ except Exception as exc:
+ error_body = (
+ f"❌ **Devin Autofix** — failed to start session\n\n"
+ f"Error: `{exc}`\n\n"
+ f"Returning this issue to human review.\n\n"
+ )
+ marker.status = "returned_to_human"
+ error_body += marker.to_marker()
+ post_comment(issue_number, error_body)
+ add_label(issue_number, "devin-autofix/returned-human")
+ print(f"::error::Failed to create Devin session: {exc}")
+ sys.exit(1)
+
+ # Update marker with session info
+ marker.devin_session_id = session["session_id"]
+ marker.devin_session_url = session["url"]
+
+ update_body = (
+ f"🤖 **Devin Autofix** — session created\n\n"
+ f"- **Devin session:** [link]({session['url']})\n"
+ f"- **Run ID:** `{run_id}`\n\n"
+ f"{marker.to_marker()}"
+ )
+ post_comment(issue_number, update_body)
+ remove_label(issue_number, "devin-autofix/returned-human")
+ add_label(issue_number, "devin-autofix/in-progress")
+
+ # Write outputs for workflow
+ if github_output := os.environ.get("GITHUB_OUTPUT", ""):
+ with open(github_output, "a") as f:
+ f.write(f"run_id={run_id}\n")
+ f.write(f"devin_session_id={session['session_id']}\n")
+ f.write(f"devin_session_url={session['url']}\n")
+
+ print(f"Session created: {session['session_id']}")
+ print(f"URL: {session['url']}")
+ print(f"Run ID: {run_id}")
+
+ # Set output as JSON for the workflow
+ output = {
+ "run_id": run_id,
+ "devin_session_id": session["session_id"],
+ "devin_session_url": session["url"],
+ "issue_type": issue_type,
+ "acceptance_criteria_source": ac_source,
+ }
+ print(f"::set-output name=result::{json.dumps(output)}")
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Deprecated GitHub Actions command</b></div>
<div id="fix">
The `::set-output` command is deprecated (GitHub deprecated it on
2022-10-11) and will be disabled. Use `$GITHUB_OUTPUT` environment file
instead. The existing `github_output` variable from line 281 can be hoisted to
write the `result` JSON output alongside the other outputs.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- scripts/devin_autofix/start_session.py
+++ scripts/devin_autofix/start_session.py
@@ -278,22 +278,21 @@
add_label(issue_number, "devin-autofix/in-progress")
# Write outputs for workflow
- if github_output := os.environ.get("GITHUB_OUTPUT", ""):
- with open(github_output, "a") as f:
- f.write(f"run_id={run_id}\n")
- f.write(f"devin_session_id={session['session_id']}\n")
- f.write(f"devin_session_url={session['url']}\n")
+ github_output = os.environ.get("GITHUB_OUTPUT", "")
+ if github_output:
+ output = {
+ "run_id": run_id,
+ "devin_session_id": session["session_id"],
+ "devin_session_url": session["url"],
+ "issue_type": issue_type,
+ "acceptance_criteria_source": ac_source,
+ }
+ with open(github_output, "a") as f:
+ f.write(f"run_id={run_id}\n")
+ f.write(f"devin_session_id={session['session_id']}\n")
+ f.write(f"devin_session_url={session['url']}\n")
+ f.write(f"result={json.dumps(output)}\n")
print(f"Session created: {session['session_id']}")
print(f"URL: {session['url']}")
print(f"Run ID: {run_id}")
-
- # Set output as JSON for the workflow
- output = {
- "run_id": run_id,
- "devin_session_id": session["session_id"],
- "devin_session_url": session["url"],
- "issue_type": issue_type,
- "acceptance_criteria_source": ac_source,
- }
- print(f"::set-output name=result::{json.dumps(output)}")
```
</div>
</details>
</div>
<small><i>Code Review Run #6cef4a</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
scripts/devin_autofix/report.py:
##########
@@ -0,0 +1,1000 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Generate analytics reports from GitHub metadata for devin-autofix runs.
+
+Produces:
+ - automation_report.html
+ - automation_runs.csv
+ - automation_runs.sqlite
+
+Usage:
+ python -m scripts.devin_autofix.report --output-dir ./report_output
+"""
+
+from __future__ import annotations
+
+import argparse
+import csv
+import json
+import os
+import pathlib
+import sqlite3
+from datetime import datetime, timezone
+from typing import Any
+
+import requests
+
+from scripts.devin_autofix.github_state import (
+ get_issue_comments,
+ GITHUB_API,
+ parse_marker,
+ REPO,
+ RunMarker,
+)
+
+AUTOFIX_LABELS = [
+ "devin-autofix/in-progress",
+ "devin-autofix/waiting-review",
+ "devin-autofix/returned-human",
+ "devin-autofix/merged",
+ "devin-autofix/reverted",
+ "devin-autofix/complete",
+]
+
+
+def _headers() -> dict[str, str]:
+ token = os.environ["GITHUB_TOKEN"]
+ return {
+ "Authorization": f"token {token}",
+ "Accept": "application/vnd.github+json",
+ "X-GitHub-Api-Version": "2022-11-28",
+ }
+
+
+def _search_issues_with_autofix_labels() -> list[dict[str, Any]]:
+ """Find all issues that have any devin-autofix label."""
+ seen: set[int] = set()
+ results: list[dict[str, Any]] = []
+
+ for label in AUTOFIX_LABELS:
+ page = 1
+ while True:
+ resp = requests.get(
+ f"{GITHUB_API}/repos/{REPO}/issues",
+ headers=_headers(),
+ params={
+ "labels": label,
+ "state": "all",
+ "per_page": str(100),
+ "page": str(page),
+ },
+ timeout=30,
+ )
+ resp.raise_for_status()
+ batch = resp.json()
+ if not batch:
+ break
+ for issue in batch:
+ if issue["number"] not in seen and "pull_request" not in issue:
+ seen.add(issue["number"])
+ results.append(issue)
+ page += 1
+
+ return results
+
+
+def _get_all_markers_for_issue(issue_number: int) -> list[RunMarker]:
+ """Get the latest marker per run_id for an issue (captures retries)."""
+ comments = get_issue_comments(issue_number)
+ latest_by_run: dict[str, RunMarker] = {}
+ for comment in comments:
+ marker = parse_marker(comment.get("body", ""))
+ if marker and marker.run_id:
+ latest_by_run[marker.run_id] = marker
+ return list(latest_by_run.values())
+
+
+def _get_pr_details(pr_number: int) -> dict[str, Any] | None:
+ """Fetch PR details."""
+ try:
+ resp = requests.get(
+ f"{GITHUB_API}/repos/{REPO}/pulls/{pr_number}",
+ headers=_headers(),
+ timeout=30,
+ )
+ resp.raise_for_status()
+ result: dict[str, Any] = resp.json()
+ return result
+ except requests.HTTPError:
+ return None
+
+
+def _get_pr_reviews(pr_number: int) -> list[dict[str, Any]]:
+ """Fetch reviews for a PR."""
+ try:
+ resp = requests.get(
+ f"{GITHUB_API}/repos/{REPO}/pulls/{pr_number}/reviews",
+ headers=_headers(),
+ params={"per_page": "100"},
+ timeout=30,
+ )
+ resp.raise_for_status()
+ result: list[dict[str, Any]] = resp.json()
+ return result
+ except requests.HTTPError:
+ return []
+
+
+def _get_pr_checks(pr_number: int) -> list[dict[str, str]]:
+ """Fetch check runs for a PR head commit."""
+ try:
+ resp = requests.get(
+ f"{GITHUB_API}/repos/{REPO}/pulls/{pr_number}",
+ headers=_headers(),
+ timeout=30,
+ )
+ resp.raise_for_status()
+ pr = resp.json()
+ head_sha = pr["head"]["sha"]
+
+ checks_resp = requests.get(
+ f"{GITHUB_API}/repos/{REPO}/commits/{head_sha}/check-runs",
+ headers=_headers(),
+ params={"per_page": "100"},
+ timeout=30,
+ )
+ checks_resp.raise_for_status()
+ check_runs = checks_resp.json().get("check_runs", [])
+ return [
+ {
+ "name": cr["name"],
+ "conclusion": cr.get("conclusion") or cr.get("status",
"pending"),
+ }
+ for cr in check_runs
+ ]
+ except requests.HTTPError:
+ return []
+
+
+def _parse_iso(ts: str | None) -> datetime | None:
+ """Parse an ISO timestamp string."""
+ if not ts:
+ return None
+ try:
+ return datetime.fromisoformat(ts.replace("Z", "+00:00"))
+ except ValueError:
+ return None
+
+
+def _seconds_between(start: datetime | None, end: datetime | None) -> int |
None:
+ if start and end:
+ return int((end - start).total_seconds())
+ return None
+
+
+def _format_duration(seconds: int | None) -> str:
+ """Format seconds into a human-readable duration with appropriate units."""
+ if seconds is None:
+ return "N/A"
+ if seconds < 60:
+ return f"{seconds}s"
+ if seconds < 3600:
+ return f"{seconds / 60:.1f} min"
+ if seconds < 86400:
+ return f"{seconds / 3600:.1f} hr"
+ if seconds < 604800:
+ return f"{seconds / 86400:.1f} days"
+ return f"{seconds / 604800:.1f} weeks"
+
+
+def _resolve_status(marker: RunMarker, issue_labels: set[str]) -> str:
+ """Determine final status from marker status and issue labels."""
+ if "devin-autofix/merged" in issue_labels:
+ return "merged"
+ if "devin-autofix/complete" in issue_labels:
+ return "merged"
+ if "devin-autofix/reverted" in issue_labels:
+ return "reverted"
+ if marker.status == "complete":
+ return "merged"
+ return marker.status
+
+
+def _classify_ci(checks: list[dict[str, str]]) -> str:
+ """Classify overall CI result from individual check conclusions."""
+ if not checks:
+ return "N/A"
+ passed_conclusions = ("success", "skipped")
+ failed = sum(1 for c in checks if c["conclusion"] == "failure")
+ if failed > 0:
+ return "failed"
+ if all(c["conclusion"] in passed_conclusions for c in checks):
+ return "passed"
+ return "pending"
+
+
+def _get_issue_type(issue: dict[str, Any]) -> str:
+ """Extract issue type label."""
+ type_labels = ("bug", "feature", "refactor", "docs")
+ return next(
+ (lbl["name"] for lbl in issue.get("labels", []) if lbl["name"] in
type_labels),
+ "unknown",
+ )
+
+
+def _build_row_for_marker(
+ marker: RunMarker,
+ issue: dict[str, Any],
+ issue_labels: set[str],
+) -> dict[str, Any]:
+ """Build a single analytics row from one marker."""
+ status = _resolve_status(marker, issue_labels)
+ pr_number = marker.pr_number
+ pr_url = marker.pr_url
+ pr_created_at: str | None = None
+ merged_at: str | None = None
+ review_outcome: str | None = None
+
+ if pr_number:
+ pr_details = _get_pr_details(pr_number)
+ if pr_details:
+ pr_created_at = pr_details.get("created_at")
+ merged_at = pr_details.get("merged_at")
+ if pr_details.get("merged"):
+ status = "merged"
+ reviews = _get_pr_reviews(pr_number)
+ for review in reversed(reviews):
+ state = review.get("state", "")
+ if state in (
+ "APPROVED",
+ "CHANGES_REQUESTED",
+ "DISMISSED",
+ ):
+ review_outcome = state.lower()
+ break
+ if review_outcome is None and status == "merged":
+ review_outcome = "approved"
+
+ checks: list[dict[str, str]] = []
+ if pr_number:
+ checks = _get_pr_checks(pr_number)
+
+ triggered_at = marker.created_at
+ time_to_pr = _seconds_between(_parse_iso(triggered_at),
_parse_iso(pr_created_at))
+ time_to_merge = _seconds_between(_parse_iso(triggered_at),
_parse_iso(merged_at))
+
+ return {
+ "run_id": marker.run_id,
+ "issue_number": issue["number"],
+ "issue_type": _get_issue_type(issue),
+ "issue_title": issue.get("title", ""),
+ "issue_url": issue["html_url"],
+ "triggered_by": marker.triggered_by,
+ "status": status,
+ "devin_session_url": marker.devin_session_url,
+ "pr_number": pr_number,
+ "pr_url": pr_url,
+ "acceptance_criteria_source": (marker.acceptance_criteria_source),
+ "reviewer_login": marker.reviewer_login,
+ "review_requested": 1 if marker.review_requested else 0,
+ "checks_total": len(checks),
+ "checks_passed": sum(1 for c in checks if c["conclusion"] ==
"success"),
+ "checks_failed": sum(1 for c in checks if c["conclusion"] ==
"failure"),
+ "ci_result": _classify_ci(checks),
+ "review_outcome": review_outcome,
+ "merged": 1 if status == "merged" else 0,
+ "return_reason": (
+ marker.review_request_error
+ if marker.status == "returned_to_human"
+ else None
+ ),
+ "triggered_at": triggered_at,
+ "pr_created_at": pr_created_at,
+ "merged_at": merged_at,
+ "reverted_at": None,
+ "time_to_pr_seconds": time_to_pr,
+ "time_to_merge_seconds": time_to_merge,
+ }
+
+
+_STATUS_PRIORITY: dict[str, int] = {
+ "merged": 0,
+ "reverted": 1,
+ "waiting_review": 2,
+ "in_progress": 3,
+ "returned_to_human": 4,
+}
+
+
+def _pick_best_row(rows: list[dict[str, Any]]) -> dict[str, Any]:
+ """Pick the most representative row for an issue.
+
+ Prefers merged > reverted > waiting_review > in_progress >
returned_to_human.
+ """
+ return min(
+ rows,
+ key=lambda r: (
+ _STATUS_PRIORITY.get(r["status"], 99),
+ 0 if r.get("pr_number") else 1,
+ ),
+ )
+
+
+def build_run_rows(issues: list[dict[str, Any]]) -> list[dict[str, Any]]:
+ """Build analytics rows from issue metadata.
+
+ Returns one row per issue, picking the most representative run.
+ """
+ rows: list[dict[str, Any]] = []
+
+ for issue in issues:
+ issue_number = issue["number"]
+ markers = _get_all_markers_for_issue(issue_number)
+ if not markers:
+ continue
+
+ issue_labels = {lbl["name"] for lbl in issue.get("labels", [])}
+
+ candidate_rows = [
+ _build_row_for_marker(marker, issue, issue_labels) for marker in
markers
+ ]
+ rows.append(_pick_best_row(candidate_rows))
+
+ return rows
+
+
+def write_sqlite(rows: list[dict[str, Any]], output_path: pathlib.Path) ->
None:
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing unit tests</b></div>
<div id="fix">
The new `report.py` module lacks unit tests. Per BITO.md rule [11730], new
tools should have comprehensive unit tests covering success paths, error
scenarios, and edge cases.
</div>
</div>
<small><i>Code Review Run #6cef4a</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
scripts/devin_autofix/github_state.py:
##########
@@ -0,0 +1,271 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Shared helpers for reading and writing devin-autofix state on GitHub
issues."""
+
+from __future__ import annotations
+
+import json
+import os
+import re
+from dataclasses import dataclass, field
+from datetime import datetime, timezone
+from typing import Any
+
+import requests
+
+MARKER_PREFIX = "<!-- devin-autofix-run:"
+MARKER_SUFFIX = "-->"
+ACTIVE_STATUSES = frozenset({"in_progress", "waiting_review"})
+REPO = os.environ.get("GITHUB_REPOSITORY", "vickyli1014/superset")
+GITHUB_API = "https://api.github.com";
+
+
+def _headers() -> dict[str, str]:
+ token = os.environ["GITHUB_TOKEN"]
+ return {
+ "Authorization": f"token {token}",
+ "Accept": "application/vnd.github+json",
+ "X-GitHub-Api-Version": "2022-11-28",
+ }
+
+
+@dataclass
+class RunMarker:
+ """Parsed hidden marker from an issue comment."""
+
+ schema_version: int = 1
+ run_id: str = ""
+ issue_number: int = 0
+ trigger_comment_id: int = 0
+ triggered_by: str = ""
+ status: str = "in_progress"
+ devin_session_id: str | None = None
+ devin_session_url: str | None = None
+ pr_number: int | None = None
+ pr_url: str | None = None
+ reviewer_login: str | None = None
+ review_requested: bool = False
+ pr_link_commented: bool = False
+ acceptance_criteria_source: str = "ai_inferred"
+ created_at: str = ""
+ updated_at: str = ""
+ review_request_error: str | None = None
+ checks: list[dict[str, str]] = field(default_factory=list)
+
+ def to_dict(self) -> dict[str, Any]:
+ return {
+ "schema_version": self.schema_version,
+ "run_id": self.run_id,
+ "issue_number": self.issue_number,
+ "trigger_comment_id": self.trigger_comment_id,
+ "triggered_by": self.triggered_by,
+ "status": self.status,
+ "devin_session_id": self.devin_session_id,
+ "devin_session_url": self.devin_session_url,
+ "pr_number": self.pr_number,
+ "pr_url": self.pr_url,
+ "reviewer_login": self.reviewer_login,
+ "review_requested": self.review_requested,
+ "pr_link_commented": self.pr_link_commented,
+ "acceptance_criteria_source": self.acceptance_criteria_source,
+ "created_at": self.created_at,
+ "updated_at": self.updated_at,
+ "review_request_error": self.review_request_error,
+ "checks": self.checks,
+ }
+
+ def to_marker(self) -> str:
+ self.updated_at = datetime.now(timezone.utc).isoformat()
+ payload = json.dumps(self.to_dict(), indent=2)
+ return f"{MARKER_PREFIX}\n{payload}\n{MARKER_SUFFIX}"
+
+
+def parse_marker(comment_body: str) -> RunMarker | None:
+ """Extract the most recent marker JSON from a comment body."""
+ pattern = re.compile(
+ re.escape(MARKER_PREFIX) + r"\s*(\{.*?\})\s*" +
re.escape(MARKER_SUFFIX),
+ re.DOTALL,
+ )
+ match = pattern.search(comment_body)
+ if not match:
+ return None
+ try:
+ data = json.loads(match.group(1))
+ except json.JSONDecodeError:
+ return None
+ marker = RunMarker()
+ for k, v in data.items():
+ if hasattr(marker, k):
+ setattr(marker, k, v)
+ return marker
+
+
+def get_issue_comments(issue_number: int) -> list[dict[str, Any]]:
+ """Fetch all comments on an issue."""
+ comments: list[dict[str, Any]] = []
+ page = 1
+ while True:
+ resp = requests.get(
+ f"{GITHUB_API}/repos/{REPO}/issues/{issue_number}/comments",
+ headers=_headers(),
+ params={"per_page": "100", "page": str(page)},
+ timeout=30,
+ )
+ resp.raise_for_status()
+ batch = resp.json()
+ if not batch:
+ break
+ comments.extend(batch)
+ page += 1
+ return comments
+
+
+def find_active_markers(issue_number: int) -> list[RunMarker]:
+ """Return markers for runs whose latest status is still active.
+
+ Multiple comments may reference the same ``run_id`` (e.g. a "run started"
+ comment followed by a "returned to human" comment). Only the **last**
+ marker per ``run_id`` determines whether the run is active.
+ """
+ comments = get_issue_comments(issue_number)
+
+ # Keep the last marker seen for each run_id (comments are chronological).
+ latest_by_run: dict[str, RunMarker] = {}
+ for comment in comments:
+ marker = parse_marker(comment.get("body", ""))
+ if marker and marker.run_id:
+ latest_by_run[marker.run_id] = marker
+
+ return [m for m in latest_by_run.values() if m.status in ACTIVE_STATUSES]
+
+
+def find_all_markers(issue_number: int) -> list[RunMarker]:
+ """Return all run markers for an issue (any status)."""
+ comments = get_issue_comments(issue_number)
+ markers: list[RunMarker] = []
+ for comment in comments:
+ marker = parse_marker(comment.get("body", ""))
+ if marker:
+ markers.append(marker)
+ return markers
+
+
+def post_comment(issue_number: int, body: str) -> dict[str, Any]:
+ """Post a comment on an issue."""
+ resp = requests.post(
+ f"{GITHUB_API}/repos/{REPO}/issues/{issue_number}/comments",
+ headers=_headers(),
+ json={"body": body},
+ timeout=30,
+ )
+ resp.raise_for_status()
+ result: dict[str, Any] = resp.json()
+ return result
+
+
+def add_label(issue_number: int, label: str) -> None:
+ """Add a label to an issue, creating it if needed."""
+ resp = requests.post(
+ f"{GITHUB_API}/repos/{REPO}/issues/{issue_number}/labels",
+ headers=_headers(),
+ json={"labels": [label]},
+ timeout=30,
+ )
+ resp.raise_for_status()
+
+
+def remove_label(issue_number: int, label: str) -> None:
+ """Remove a label from an issue (no-op if absent)."""
+ resp = requests.delete(
+ f"{GITHUB_API}/repos/{REPO}/issues/{issue_number}/labels/{label}",
+ headers=_headers(),
+ timeout=30,
+ )
+ if resp.status_code != 404:
+ resp.raise_for_status()
+
+
+def get_issue(issue_number: int) -> dict[str, Any]:
+ """Fetch a single issue."""
+ resp = requests.get(
+ f"{GITHUB_API}/repos/{REPO}/issues/{issue_number}",
+ headers=_headers(),
+ timeout=30,
+ )
+ resp.raise_for_status()
+ result: dict[str, Any] = resp.json()
+ return result
+
+
+def classify_issue_type(issue: dict[str, Any]) -> str:
+ """Derive issue type from labels."""
+ label_names = [lbl["name"].lower() for lbl in issue.get("labels", [])]
+ for name in label_names:
+ if "bug" in name:
+ return "bug"
+ if "feature" in name or "enhancement" in name:
+ return "feature"
+ if "refactor" in name:
+ return "refactor"
+ if "docs" in name or "documentation" in name:
+ return "docs"
+ return "unknown"
+
+
+def detect_acceptance_criteria(issue_body: str | None) -> str:
+ """Check whether the issue body contains explicit acceptance criteria."""
+ if not issue_body:
+ return "ai_inferred"
+ lower = issue_body.lower()
+ keywords = ["acceptance criteria", "expected behavior", "expected result",
"should"]
+ for kw in keywords:
+ if kw in lower:
+ return "issue"
+ return "ai_inferred"
+
+
+def get_issues_with_label(label: str, state: str = "open") -> list[dict[str,
Any]]:
+ """Fetch issues that have a given label.
+
+ ``state`` can be ``"open"``, ``"closed"``, or ``"all"``.
+ """
+ issues: list[dict[str, Any]] = []
+ page = 1
+ while True:
+ resp = requests.get(
+ f"{GITHUB_API}/repos/{REPO}/issues",
+ headers=_headers(),
+ params={
+ "labels": label,
+ "state": state,
+ "per_page": str(100),
+ "page": str(page),
+ },
+ timeout=30,
+ )
+ resp.raise_for_status()
+ batch = resp.json()
+ if not batch:
+ break
+ issues.extend(batch)
+ page += 1
+ return issues
+
+
+def get_open_issues_with_label(label: str) -> list[dict[str, Any]]:
+ """Fetch open issues that have a given label."""
+ return get_issues_with_label(label, state="open")
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing unit tests for new module</b></div>
<div id="fix">
Per BITO.md rule [11730] and [11731], new modules must include unit tests
covering success paths, error scenarios, and edge cases. No test file was found
for `github_state.py`. Tests should follow the established pattern of other MCP
tool tests in `tests/unit_tests/mcp_service/*/tool/`.
</div>
</div>
<div id="suggestion">
<div id="issue"><b>Duplicate header function across files</b></div>
<div id="fix">
The `_headers()` function in `github_state.py` duplicates
`_github_headers()` in `monitor_sessions.py`. Consider extracting this logic
into a shared utility module and importing it in both files to avoid
duplication.
</div>
</div>
<small><i>Code Review Run #6cef4a</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
scripts/devin_autofix/reviewers.py:
##########
@@ -0,0 +1,111 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Reviewer request logic for devin-autofix PRs."""
+
+from __future__ import annotations
+
+import os
+from typing import Any
+
+import requests
+
+from scripts.devin_autofix.github_state import GITHUB_API, REPO
+
+
+def _headers() -> dict[str, str]:
+ token = os.environ["GITHUB_TOKEN"]
+ return {
+ "Authorization": f"token {token}",
+ "Accept": "application/vnd.github+json",
+ "X-GitHub-Api-Version": "2022-11-28",
+ }
+
+
+def request_review(
+ pr_number: int,
+ triggered_by: str,
+) -> dict[str, Any]:
+ """Request a review from the trigger commenter on a PR.
+
+ Returns a dict with:
+ - review_requested: bool
+ - reviewer_login: str | None
+ - error: str | None
+ """
+ # Get PR to check author
+ resp = requests.get(
+ f"{GITHUB_API}/repos/{REPO}/pulls/{pr_number}",
+ headers=_headers(),
+ timeout=30,
+ )
+ resp.raise_for_status()
+ pr = resp.json()
+ pr_author = pr["user"]["login"]
+
+ # Skip if trigger commenter is the PR author
+ if triggered_by == pr_author:
+ print(
+ f" Skipping reviewer request: trigger commenter "
+ f"({triggered_by}) is the PR author"
+ )
+ return {
+ "review_requested": False,
+ "reviewer_login": None,
+ "error": None,
+ }
+
+ # Request review
+ try:
+ review_resp = requests.post(
+ f"{GITHUB_API}/repos/{REPO}/pulls/{pr_number}/requested_reviewers",
+ headers=_headers(),
+ json={"reviewers": [triggered_by]},
+ timeout=30,
+ )
+ review_resp.raise_for_status()
+ print(f" Review requested from {triggered_by} on PR #{pr_number}")
+ return {
+ "review_requested": True,
+ "reviewer_login": triggered_by,
+ "error": None,
+ }
+ except requests.HTTPError as exc:
+ error_msg = f"Failed to request review from {triggered_by}: {exc}"
+ print(f" {error_msg}")
+
+ # Post a best-effort comment on the PR
+ try:
+ comment_body = (
+ f"⚠️ **Devin Autofix** — could not request review\n\n"
+ f"Attempted to request review from @{triggered_by} but "
+ f"the request failed: `{exc}`\n\n"
+ f"Please review this PR manually."
+ )
+ requests.post(
+ f"{GITHUB_API}/repos/{REPO}/issues/{pr_number}/comments",
+ headers=_headers(),
+ json={"body": comment_body},
+ timeout=30,
+ )
+ except Exception as comment_exc:
+ print(f" Failed to post fallback comment: {comment_exc}")
+
+ return {
+ "review_requested": False,
+ "reviewer_login": triggered_by,
+ "error": error_msg,
+ }
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing unit tests for reviewers.py</b></div>
<div id="fix">
The new `request_review` function lacks unit tests. Per project guidelines,
new tools should include tests covering success paths, error scenarios, and
edge cases. No test file exists under `tests/` for this module.
</div>
</div>
<small><i>Code Review Run #6cef4a</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
tests/unit_tests/extensions/test_api.py:
##########
@@ -0,0 +1,226 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from typing import Any
+from unittest.mock import patch
+
+import pytest
+import rison
+from flask.testing import FlaskClient
+from superset_core.extensions.types import Manifest, ManifestFrontend
+
+from superset.extensions.types import LoadedExtension
+
+EXTENSIONS_LIST_URL = "/api/v1/extensions/"
+
+_ENABLE_EXTENSIONS_CONFIG = {
+ "FEATURE_FLAGS": {"ENABLE_EXTENSIONS": True},
+ "LOCAL_EXTENSIONS": [],
+}
+
+
+def _make_extension(
+ publisher: str,
+ name: str,
+ display_name: str,
+ version: str = "1.0.0",
+ description: str | None = None,
+) -> LoadedExtension:
+ ext_id = f"{publisher}.{name}"
+ manifest = Manifest(
+ id=ext_id,
+ publisher=publisher,
+ name=name,
+ displayName=display_name,
+ version=version,
+ description=description,
+ frontend=ManifestFrontend(
+ remoteEntry="remoteEntry.js",
+ moduleFederationName=f"{publisher}_{name}",
+ ),
+ )
+ return LoadedExtension(
+ id=ext_id,
+ name=name,
+ manifest=manifest,
+ frontend={},
+ backend={},
+ version=version,
+ source_base_path="/fake/extensions/path", # noqa: S108
+ )
+
+
+FAKE_EXTENSIONS: dict[str, LoadedExtension] = {
+ "acme.charts": _make_extension(
+ "acme", "charts", "ACME Charts", description="Chart extensions by ACME"
+ ),
+ "acme.filters": _make_extension(
+ "acme", "filters", "ACME Filters", description="Filter tools"
+ ),
+ "globex.dashboards": _make_extension(
+ "globex", "dashboards", "Globex Dashboards", description="Dashboard
widgets"
+ ),
+}
+
+
+def _mock_get_extensions() -> dict[str, LoadedExtension]:
+ return dict(FAKE_EXTENSIONS)
+
+
+def _get_json(client: FlaskClient, url: str) -> dict[str, Any]:
+ rv = client.get(url)
+ return rv.get_json()
+
+
[email protected]("app", [_ENABLE_EXTENSIONS_CONFIG], indirect=True)
+def test_get_list_no_q_returns_all(client: FlaskClient, full_api_access: None)
-> None:
+ """GET /api/v1/extensions/ without q returns all extensions."""
+ with patch(
+ "superset.extensions.api.get_extensions",
+ side_effect=_mock_get_extensions,
+ ):
+ data = _get_json(client, EXTENSIONS_LIST_URL)
+ assert data["count"] == 3
+ assert len(data["result"]) == 3
+
+
[email protected]("app", [_ENABLE_EXTENSIONS_CONFIG], indirect=True)
+def test_get_list_q_filter_by_name(client: FlaskClient, full_api_access: None)
-> None:
+ """GET /api/v1/extensions/?q=... filters by name field."""
+ q = rison.dumps({"filters": [{"col": "name", "opr": "eq", "value":
"charts"}]})
+ with patch(
+ "superset.extensions.api.get_extensions",
+ side_effect=_mock_get_extensions,
+ ):
+ data = _get_json(client, f"{EXTENSIONS_LIST_URL}?q={q}")
+ assert data["count"] == 1
+ assert data["result"][0]["name"] == "charts"
+
+
[email protected]("app", [_ENABLE_EXTENSIONS_CONFIG], indirect=True)
+def test_get_list_q_filter_by_publisher(
+ client: FlaskClient, full_api_access: None
+) -> None:
+ """GET /api/v1/extensions/?q=... filters by publisher field."""
+ q = rison.dumps({"filters": [{"col": "publisher", "opr": "eq", "value":
"acme"}]})
+ with patch(
+ "superset.extensions.api.get_extensions",
+ side_effect=_mock_get_extensions,
+ ):
+ data = _get_json(client, f"{EXTENSIONS_LIST_URL}?q={q}")
+ assert data["count"] == 2
+ ids = {ext["id"] for ext in data["result"]}
+ assert ids == {"acme.charts", "acme.filters"}
+
+
[email protected]("app", [_ENABLE_EXTENSIONS_CONFIG], indirect=True)
+def test_get_list_q_search_text(client: FlaskClient, full_api_access: None) ->
None:
+ """GET /api/v1/extensions/?q=... supports text search across
name/description."""
+ q = rison.dumps({"search": "dashboard"})
+ with patch(
+ "superset.extensions.api.get_extensions",
+ side_effect=_mock_get_extensions,
+ ):
+ data = _get_json(client, f"{EXTENSIONS_LIST_URL}?q={q}")
+ assert data["count"] == 1
+ assert data["result"][0]["id"] == "globex.dashboards"
+
+
[email protected]("app", [_ENABLE_EXTENSIONS_CONFIG], indirect=True)
+def test_get_list_q_search_case_insensitive(
+ client: FlaskClient, full_api_access: None
+) -> None:
+ """Search is case-insensitive."""
+ q = rison.dumps({"search": "ACME"})
+ with patch(
+ "superset.extensions.api.get_extensions",
+ side_effect=_mock_get_extensions,
+ ):
+ data = _get_json(client, f"{EXTENSIONS_LIST_URL}?q={q}")
+ assert data["count"] == 2
+
+
[email protected]("app", [_ENABLE_EXTENSIONS_CONFIG], indirect=True)
+def test_get_list_q_no_matches(client: FlaskClient, full_api_access: None) ->
None:
+ """q that matches nothing returns empty result."""
+ q = rison.dumps({"search": "nonexistent"})
+ with patch(
+ "superset.extensions.api.get_extensions",
+ side_effect=_mock_get_extensions,
+ ):
+ data = _get_json(client, f"{EXTENSIONS_LIST_URL}?q={q}")
+ assert data["count"] == 0
+ assert data["result"] == []
+
+
[email protected]("app", [_ENABLE_EXTENSIONS_CONFIG], indirect=True)
+def test_get_list_q_invalid_rison(client: FlaskClient, full_api_access: None)
-> None:
+ """Invalid rison q returns 400."""
+ with patch(
+ "superset.extensions.api.get_extensions",
+ side_effect=_mock_get_extensions,
+ ):
+ rv = client.get(f"{EXTENSIONS_LIST_URL}?q=((invalid")
+ assert rv.status_code == 400
+
+
[email protected]("app", [_ENABLE_EXTENSIONS_CONFIG], indirect=True)
+def test_get_list_q_invalid_filter_col(
+ client: FlaskClient, full_api_access: None
+) -> None:
+ """Filtering on an unsupported column returns 400."""
+ q = rison.dumps({"filters": [{"col": "secret_field", "opr": "eq", "value":
"x"}]})
+ with patch(
+ "superset.extensions.api.get_extensions",
+ side_effect=_mock_get_extensions,
+ ):
+ rv = client.get(f"{EXTENSIONS_LIST_URL}?q={q}")
+ assert rv.status_code == 400
+
+
[email protected]("app", [_ENABLE_EXTENSIONS_CONFIG], indirect=True)
+def test_get_list_q_pagination(client: FlaskClient, full_api_access: None) ->
None:
+ """q supports page and page_size for pagination."""
+ q = rison.dumps({"page": 0, "page_size": 2})
+ with patch(
+ "superset.extensions.api.get_extensions",
+ side_effect=_mock_get_extensions,
+ ):
+ data = _get_json(client, f"{EXTENSIONS_LIST_URL}?q={q}")
+ assert len(data["result"]) == 2
+ assert data["count"] == 3
+
+
[email protected]("app", [_ENABLE_EXTENSIONS_CONFIG], indirect=True)
+def test_get_list_response_shape_unchanged(
+ client: FlaskClient, full_api_access: None
+) -> None:
+ """Response shape has result and count keys with correct types."""
+ with patch(
+ "superset.extensions.api.get_extensions",
+ side_effect=_mock_get_extensions,
+ ):
+ data = _get_json(client, EXTENSIONS_LIST_URL)
+ assert "result" in data
+ assert "count" in data
+ assert isinstance(data["result"], list)
+ assert isinstance(data["count"], int)
+ for ext in data["result"]:
+ assert "id" in ext
+ assert "name" in ext
+ assert "version" in ext
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete response shape assertions</b></div>
<div id="fix">
The test `test_get_list_response_shape_unchanged` checks only 3 of 7+ fields
returned by `build_extension_data`. Missing: `description`, `dependencies`,
`remoteEntry`, and `moduleFederationName`. This contradicts the test's intent
of verifying the complete response shape and could hide bugs where new fields
are silently dropped.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
--- tests/unit_tests/extensions/test_api.py (lines 210-226) ---
210: def test_get_list_response_shape_unchanged(
211: client: FlaskClient, full_api_access: None
212: ) -> None:
213: """Response shape has result and count keys with correct types."""
214: with patch(
215: "superset.extensions.api.get_extensions",
216: side_effect=_mock_get_extensions,
217: ):
218: data = _get_json(client, EXTENSIONS_LIST_URL)
219: assert "result" in data
220: assert "count" in data
221: assert isinstance(data["result"], list)
222: assert isinstance(data["count"], int)
223: for ext in data["result"]:
224: assert "id" in ext
225: assert "name" in ext
226: assert "version" in ext
+ for ext in data["result"]:
+ assert "description" in ext
+ assert "dependencies" in ext
+ # frontend fields present since all test fixtures have
ManifestFrontend
+ assert "remoteEntry" in ext
+ assert "moduleFederationName" in ext
```
</div>
</details>
</div>
<small><i>Code Review Run #6cef4a</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/packages/superset-ui-core/test/chart/components/SuperChart.test.tsx:
##########
@@ -405,8 +414,7 @@ describe('SuperChart', () => {
});
});
- /* oxlint-disable-next-line jest/no-disabled-tests */
- test.skip('works when width and height are percent', async () => {
+ test('works when width and height are percent', async () => {
const wrapper = createSizedWrapper();
document.body.appendChild(wrapper);
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Test Memory Leak - DOM Not Cleaned</b></div>
<div id="fix">
Add an `afterEach` or `afterAll` cleanup step to remove the `wrapper`
element appended to `document.body`, e.g., by storing the wrapper in a
higher-scope variable and calling `wrapper.remove()` or
`document.body.removeChild(wrapper)` to avoid DOM leaks.
</div>
</div>
<small><i>Code Review Run #6cef4a</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.test.tsx:
##########
@@ -166,12 +166,17 @@ test('should call refreshChart when SliceHeader calls
forceRefresh', () => {
expect(refreshChart).toHaveBeenCalled();
});
-/* oxlint-disable-next-line jest/no-disabled-tests */
-test.skip('should call changeFilter when ChartContainer calls changeFilter',
() => {
- const mockChangeFilter = jest.fn();
- const wrapper = setup({ changeFilter: mockChangeFilter }) as any;
- wrapper.instance().changeFilter();
- expect((mockChangeFilter as any).callCount).toBe(1);
+test('should call changeFilter when ChartContainer calls addFilter', () => {
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Misleading test name</b></div>
<div id="fix">
Test name states 'ChartContainer calls addFilter' but `addFilter` is defined
in the Chart component (line 290) and passed as a prop to ChartContainer. The
call chain is: test → addFilter prop → Chart's addFilter callback →
changeFilter. This misleading name could cause future maintainers to
misunderstand the component relationship.
</div>
</div>
<small><i>Code Review Run #6cef4a</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/AdhocFilterEditPopover.test.tsx:
##########
@@ -25,6 +25,23 @@ import AdhocFilterEditPopover from '.';
import AdhocFilter from '../AdhocFilter';
import { Clauses, ExpressionTypes } from '../types';
+jest.mock('src/components/SQLEditorWithValidation', () => ({
+ __esModule: true,
+ default: ({
+ value,
+ onChange,
+ }: {
+ value: string;
+ onChange: (v: string) => void;
+ }) => (
+ <textarea
+ data-test="sql-editor"
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Wrong attribute for testId lookup</b></div>
<div id="fix">
The mock's `data-test` attribute will not be found by
`screen.getByTestId('sql-editor')` on line 148. React Testing Library's
`getByTestId` queries elements using `data-testid` by default (verified via
official docs). Compare with ColumnSelectPopover.test.tsx:47 which correctly
uses `data-testid`. This will cause the test to fail with a 'Unable to find
element' error at runtime.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
```
---
a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/AdhocFilterEditPopover.test.tsx
+++
b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/AdhocFilterEditPopover.test.tsx
@@ -37,3 +37,3 @@
defaultValue={value}
- data-test="sql-editor"
+ data-testid="sql-editor"
onChange={e => onChange(e.target.value)}
```
</div>
</details>
</div>
<small><i>Code Review Run #6cef4a</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
