diff --git a/.gitignore b/.gitignore index 48c37ac..b637436 100644 --- a/.gitignore +++ b/.gitignore @@ -19,8 +19,12 @@ docs/plans/ nboot-spec.json .hypothesis/ .worktrees/ +.claude-flow/ benchmarks/output/ !benchmarks/output/.gitkeep +benchmarks/martian/output/ +benchmarks/martian/output-*/ +!benchmarks/martian/output/.gitkeep # Zensical build output site/ diff --git a/.secrets.baseline b/.secrets.baseline index 3f9fecb..a0078c6 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -173,6 +173,15 @@ "line_number": 58 } ], + "benchmarks/martian/config.py": [ + { + "type": "Hex High Entropy String", + "filename": "benchmarks/martian/config.py", + "hashed_secret": "e6a37dac608a721d5d88c51c6af2535e403d405c", + "is_verified": false, + "line_number": 10 + } + ], "scripts/check_test_parity.py": [ { "type": "Secret Keyword", @@ -182,6 +191,15 @@ "line_number": 38 } ], + "tests/test_bench_martian_config.py": [ + { + "type": "Hex High Entropy String", + "filename": "tests/test_bench_martian_config.py", + "hashed_secret": "e6a37dac608a721d5d88c51c6af2535e403d405c", + "is_verified": false, + "line_number": 9 + } + ], "tests/test_grippy_agent_evolution.py": [ { "type": "Secret Keyword", @@ -281,5 +299,5 @@ } ] }, - "generated_at": "2026-03-12T03:37:04Z" + "generated_at": "2026-03-13T18:52:34Z" } diff --git a/benchmarks/martian/README.md b/benchmarks/martian/README.md new file mode 100644 index 0000000..96004bb --- /dev/null +++ b/benchmarks/martian/README.md @@ -0,0 +1,11 @@ +# Grippy Martian Benchmark Harness + +Runs Grippy against the [withmartian/code-review-benchmark](https://github.com/withmartian/code-review-benchmark) +50-PR golden dataset. + +**This is a dev-path preflight harness.** For public leaderboard claims, +use the Martian-native public path (see design doc). + +## Quick Start + +See design doc: `docs/plans/2026-03-12-martian-benchmark-harness-design.md` diff --git a/benchmarks/martian/__init__.py b/benchmarks/martian/__init__.py new file mode 100644 index 0000000..8f8138a --- /dev/null +++ b/benchmarks/martian/__init__.py @@ -0,0 +1,2 @@ +# SPDX-License-Identifier: MIT +"""Grippy benchmark harness for the Martian code-review-benchmark.""" diff --git a/benchmarks/martian/config.py b/benchmarks/martian/config.py new file mode 100644 index 0000000..da2713b --- /dev/null +++ b/benchmarks/martian/config.py @@ -0,0 +1,90 @@ +# SPDX-License-Identifier: MIT +"""Benchmark run configuration — frozen for headline score.""" + +from __future__ import annotations + +import os +from dataclasses import dataclass, field +from pathlib import Path + +MARTIAN_COMMIT_SHA = "012d68202e56280855b85abb956410086008a7b2" + +_HERE = Path(__file__).resolve().parent + + +@dataclass(frozen=True) +class BenchConfig: + """Immutable benchmark run configuration.""" + + model_id: str = "nemotron-3-nano-30b-a3b" + transport: str = "local" + base_url: str = "http://localhost:1234/v1" + api_key: str = "lm-studio" + profile: str = "security" + mode: str = "pr_review" + extract_model: str = "claude-sonnet-4-5-20250929" + judge_model: str = "claude-sonnet-4-5-20250929" + golden_dir: Path = field(default_factory=lambda: _HERE / "golden_comments") + output_dir: Path = field(default_factory=lambda: _HERE / "output") + + @classmethod + def from_env(cls) -> BenchConfig: + """Build config from GRIPPY_* environment variables.""" + return cls( + model_id=os.environ.get("GRIPPY_MODEL_ID", cls.model_id), + transport=os.environ.get("GRIPPY_TRANSPORT", cls.transport), + base_url=os.environ.get("GRIPPY_BASE_URL", cls.base_url), + api_key=os.environ.get("GRIPPY_API_KEY", cls.api_key), + profile=os.environ.get("GRIPPY_PROFILE", cls.profile), + mode=os.environ.get("GRIPPY_MODE", cls.mode), + ) + + def stamp(self) -> dict: + """Return full provenance dict for embedding in output files. + + Includes everything needed to reproduce this run: Grippy version, + harness git SHA, vendored prompt checksums, judge/extract model IDs, + and all config values. Two runs with identical stamps produce + identical results (modulo LLM non-determinism). + """ + import hashlib + import subprocess + + # Grippy package version + try: + from importlib.metadata import version as pkg_version + + grippy_ver = pkg_version("grippy-mcp") + except Exception: + grippy_ver = "unknown" + + # Harness git SHA (full, not truncated) + try: + harness_sha = subprocess.check_output( + ["git", "rev-parse", "HEAD"], + cwd=_HERE, + text=True, + timeout=5, + ).strip() + except Exception: + harness_sha = "unknown" + + # Vendored prompt checksums (full SHA-256, not truncated) + prompt_checksums = {} + prompts_dir = _HERE / "prompts" + for p in sorted(prompts_dir.glob("*.txt")): + h = hashlib.sha256(p.read_bytes()).hexdigest() + prompt_checksums[p.name] = h + + return { + "grippy_version": grippy_ver, + "harness_git_sha": harness_sha, + "model_id": self.model_id, + "transport": self.transport, + "profile": self.profile, + "mode": self.mode, + "martian_commit": MARTIAN_COMMIT_SHA, + "extract_model": self.extract_model, + "judge_model": self.judge_model, + "prompt_checksums": prompt_checksums, + } diff --git a/benchmarks/martian/extract.py b/benchmarks/martian/extract.py new file mode 100644 index 0000000..e149024 --- /dev/null +++ b/benchmarks/martian/extract.py @@ -0,0 +1,135 @@ +# SPDX-License-Identifier: MIT +"""Step 3: Extract discrete candidates from Grippy review comments. + +Mirrors Martian's step2_extract_comments.py: +- Inline comments (file + line) become candidates directly. +- General comments are LLM-extracted into discrete issues. + +Prompts vendored verbatim from Martian @ 012d682. +""" + +from __future__ import annotations + +import json +import sys +from collections.abc import Callable +from pathlib import Path + +from benchmarks.martian.config import BenchConfig + +# --- Vendored Martian prompts (loaded from files, not inline) --- +_PROMPTS_DIR = Path(__file__).resolve().parent / "prompts" + +EXTRACT_SYSTEM_PROMPT = (_PROMPTS_DIR / "extract_system.txt").read_text().strip() +EXTRACT_USER_PROMPT = (_PROMPTS_DIR / "extract.txt").read_text().strip() + +# --- End vendored prompts --- + + +def inline_to_candidate(inline: dict) -> dict: + """Convert an inline comment to a candidate (direct, no extraction).""" + return { + "text": inline["body"], + "path": inline.get("path"), + "line": inline.get("line"), + "source": "inline", + } + + +def _llm_extract_default(text: str) -> list[str]: + """Extract issues from general comment text using Claude.""" + import anthropic + + client = anthropic.Anthropic() + resp = client.messages.create( + model="claude-sonnet-4-5-20250929", + max_tokens=2048, + temperature=0.0, + system=EXTRACT_SYSTEM_PROMPT, + messages=[{"role": "user", "content": EXTRACT_USER_PROMPT.format(comment=text)}], + ) + content = resp.content[0].text.strip() + # Strip markdown code fences if present (Claude sometimes wraps JSON) + if content.startswith("```"): + content = content.split("```")[1] + if content.startswith("json"): + content = content[4:] + content = content.strip() + parsed = json.loads(content) + return parsed.get("issues", []) + + +def extract_candidates_for_pr( + comments: dict, + llm_extract_fn: Callable[[str], list[str]] | None = _llm_extract_default, +) -> list[dict]: + """Extract candidates from a PR's comments. + + Args: + comments: dict with "inline" and "general" keys from run_grippy.py + llm_extract_fn: function to extract issues from general text. + Pass None to skip general extraction (when no general comments). + """ + candidates = [] + + # Inline findings → direct candidates + for inline in comments.get("inline", []): + candidates.append(inline_to_candidate(inline)) + + # General findings → LLM extraction + general_texts = comments.get("general", []) + if general_texts and llm_extract_fn is not None: + combined = "\n\n---\n\n".join(general_texts) + issues = llm_extract_fn(combined) + for issue_text in issues: + candidates.append( + { + "text": issue_text, + "path": None, + "line": None, + "source": "extracted", + } + ) + + return candidates + + +def extract_all(config: BenchConfig | None = None) -> None: + """Extract candidates for all reviewed PRs. + + Writes per-PR status to extract_manifest.json for unified failure accounting. + """ + config = config or BenchConfig() + comment_dir = config.output_dir / "comments" + cand_dir = config.output_dir / "candidates" + cand_dir.mkdir(parents=True, exist_ok=True) + + manifest = [] + for comment_path in sorted(comment_dir.glob("*.json")): + slug = comment_path.stem + cand_path = cand_dir / f"{slug}.json" + + try: + comments = json.loads(comment_path.read_text()) + n_general = len(comments.get("general", [])) + + print(f"{slug}: {len(comments.get('inline', []))} inline, {n_general} general") + + extract_fn = _llm_extract_default if n_general > 0 else None + candidates = extract_candidates_for_pr(comments, llm_extract_fn=extract_fn) + + cand_path.write_text(json.dumps(candidates, indent=2)) + print(f" → {len(candidates)} candidates") + manifest.append({"pr": slug, "status": "ok", "candidates": len(candidates)}) + except Exception as e: + print(f" ERROR: {e}", file=sys.stderr) + manifest.append({"pr": slug, "status": "failed", "reason": str(e)}) + + # Write extract manifest for unified failure accounting + manifest_path = config.output_dir / "extract_manifest.json" + manifest_path.write_text(json.dumps(manifest, indent=2)) + print(f"Done. Candidates in {cand_dir}") + + +if __name__ == "__main__": + extract_all() diff --git a/benchmarks/martian/fetch_diffs.py b/benchmarks/martian/fetch_diffs.py new file mode 100644 index 0000000..b3a752b --- /dev/null +++ b/benchmarks/martian/fetch_diffs.py @@ -0,0 +1,99 @@ +# SPDX-License-Identifier: MIT +"""Step 1: Fetch PR diffs from GitHub for benchmark corpus.""" + +from __future__ import annotations + +import json +import os +import sys +import time +from pathlib import Path +from urllib.parse import urlparse + +import httpx + +from benchmarks.martian.config import BenchConfig + + +def parse_golden_prs(golden_dir: Path) -> list[dict]: + """Parse all golden comment files and extract PR metadata.""" + prs: list[dict] = [] + for path in sorted(golden_dir.glob("*.json")): + data = json.loads(path.read_text()) + for entry in data: + url = entry["url"] + parts = urlparse(url).path.strip("/").split("/") + # parts: [owner, repo, "pull", number] + prs.append( + { + "owner": parts[0], + "repo": parts[1], + "pr_number": int(parts[3]), + "pr_title": entry["pr_title"], + "golden_url": url, + "golden_file": path.name, + "golden_comments": entry.get("comments", []), + } + ) + return prs + + +def fetch_diff( + *, + owner: str, + repo: str, + pr_number: int, + output_dir: Path, + token: str, +) -> str: + """Fetch a PR diff from GitHub API. Returns diff text. Caches to disk.""" + cache_path = output_dir / f"{repo}_PR{pr_number}.diff" + if cache_path.exists(): + return cache_path.read_text() + + url = f"https://api.github.com/repos/{owner}/{repo}/pulls/{pr_number}" + headers = { + "Authorization": f"Bearer {token}", + "Accept": "application/vnd.github.v3.diff", + } + resp = httpx.get(url, headers=headers, timeout=30, follow_redirects=True) + resp.raise_for_status() + + diff_text = resp.text + output_dir.mkdir(parents=True, exist_ok=True) + cache_path.write_text(diff_text) + return diff_text + + +def fetch_all(config: BenchConfig | None = None) -> None: + """Fetch diffs for all golden PRs.""" + config = config or BenchConfig() + token = os.environ.get("GITHUB_TOKEN", "") + if not token: + print("ERROR: GITHUB_TOKEN env var required", file=sys.stderr) + sys.exit(1) + + prs = parse_golden_prs(config.golden_dir) + diff_dir = config.output_dir / "diffs" + diff_dir.mkdir(parents=True, exist_ok=True) + + for i, pr in enumerate(prs, 1): + cache_path = diff_dir / f"{pr['repo']}_PR{pr['pr_number']}.diff" + status = "cached" if cache_path.exists() else "fetching" + print(f"[{i}/{len(prs)}] {pr['repo']}#{pr['pr_number']} — {status}") + + fetch_diff( + owner=pr["owner"], + repo=pr["repo"], + pr_number=pr["pr_number"], + output_dir=diff_dir, + token=token, + ) + if status == "fetching": + time.sleep(1) # respect rate limits + + print(f"Done. {len(prs)} diffs in {diff_dir}") + + +if __name__ == "__main__": + fetch_all() diff --git a/benchmarks/martian/golden_comments/SOURCE.md b/benchmarks/martian/golden_comments/SOURCE.md new file mode 100644 index 0000000..1278aa7 --- /dev/null +++ b/benchmarks/martian/golden_comments/SOURCE.md @@ -0,0 +1,4 @@ +Vendored from withmartian/code-review-benchmark (MIT license). +Commit: 012d68202e56280855b85abb956410086008a7b2 +Date: 2026-03-05 +Files: sentry.json, grafana.json, cal_dot_com.json, discourse.json, keycloak.json diff --git a/benchmarks/martian/golden_comments/cal_dot_com.json b/benchmarks/martian/golden_comments/cal_dot_com.json new file mode 100644 index 0000000..421be7c --- /dev/null +++ b/benchmarks/martian/golden_comments/cal_dot_com.json @@ -0,0 +1,186 @@ +[ + { + "pr_title": "Async import of the appStore packages", + "url": "https://github.com/calcom/cal.com/pull/8087", + "comments": [ + { + "comment": "Consider adding try-catch around the await to handle import failures gracefully", + "severity": "Low" + }, + { + "comment": "The code uses forEach with async callbacks, which causes asynchronous operations (e.g., calendar/video event deletions, payment refunds) to run concurrently without being awaited. This 'fire-and-forget' behavior leads to unhandled promise rejections, race conditions, and incomplete cleanup, as surrounding try-catch blocks cannot properly handle errors from these unawaited promises. Replace forEach with for...of loops or Promise.all() with map() to ensure proper sequential execution and error handling.", + "severity": "Critical" + } + ] + }, + { + "pr_title": "feat: 2fa backup codes", + "url": "https://github.com/calcom/cal.com/pull/10600", + "comments": [ + { + "comment": "The exported function TwoFactor handles backup codes and is in BackupCode.tsx. Inconsistent naming.", + "severity": "Low" + }, + { + "comment": "Error message mentions 'backup code login' but this is a disable endpoint, not login", + "severity": "Low" + }, + { + "comment": "Backup code validation is case-sensitive due to the use of indexOf(). This causes validation to fail if a user enters uppercase hex characters, as backup codes should be case-insensitive for a better user experience.", + "severity": "Medium" + }, + { + "comment": "Because backupCodes are decrypted and mutated in memory before being written back, two concurrent login requests using the same backupCode could both pass this check and update, so a single backup code may effectively be accepted more than once if used concurrently, weakening the intended one-time-use semantics.", + "severity": "High" + } + ] + }, + { + "pr_title": "fix: handle collective multiple host on destinationCalendar", + "url": "https://github.com/calcom/cal.com/pull/10967", + "comments": [ + { + "comment": "Potential null reference if mainHostDestinationCalendar is undefined if evt.destinationCalendar is null or an empty array ", + "severity": "High" + }, + { + "comment": "The optional chaining on mainHostDestinationCalendar?.integration is redundant since you already check mainHostDestinationCalendar in the ternary condition.", + "severity": "Low" + }, + { + "comment": "Logic error: when externalCalendarId is provided, you're searching for a calendar where externalId === externalCalendarId, but this will always fail since you're looking for a calendar that matches itself. Should likely find by credentialId or use different logic.", + "severity": "High" + }, + { + "comment": "Logic inversion in organization creation: The slug property is now conditionally set when IS_TEAM_BILLING_ENABLED is true, instead of when it's false as originally intended. This change, combined with requestedSlug still being set when IS_TEAM_BILLING_ENABLED is true, results in both properties being set when billing is enabled, and neither when disabled", + "severity": "Medium" + }, + { + "comment": "The Calendar interface now requires createEvent(event, credentialId), but some implementations (e.g., Lark/Office365) still declare createEvent(event) only—this breaks the interface contract (also applies to other locations in the PR).", + "severity": "Low" + } + ] + }, + { + "pr_title": "feat: convert InsightsBookingService to use Prisma.sql raw queries", + "url": "https://github.com/calcom/cal.com/pull/22345", + "comments": [ + { + "comment": "In getBaseConditions(), the else if (filterConditions) and final else branches are unreachable. This is because getAuthorizationConditions() always returns a non-null Prisma.Sql object, making authConditions always truthy, which means only the first two if/else if conditions are ever evaluated.", + "severity": "Low" + }, + { + "comment": "Fetching userIdsFromOrg only when teamsFromOrg.length > 0 can exclude org-level members for orgs without child teams; consider deriving from teamIds (which includes orgId) or removing the guard so org-only orgs still include member user bookings.", + "severity": "Medium" + } + ] + }, + { + "pr_title": "Comprehensive workflow reminder management for booking lifecycle events", + "url": "https://github.com/calcom/cal.com/pull/7232", + "comments": [ + { + "comment": "Asynchronous functions deleteScheduledEmailReminder and deleteScheduledSMSReminder are called without await inside forEach loops. This occurs during booking rescheduling/cancellation, and workflow/workflow step deletion/updates. Consequently, scheduled workflow reminders may not be reliably cancelled, potentially leaving them active.", + "severity": "Medium" + }, + { + "comment": "When immediateDelete is true, the deleteScheduledEmailReminder function cancels the SendGrid email but fails to delete the corresponding WorkflowReminder record from the database. This creates orphaned database entries and is inconsistent with the immediateDelete: false path, which marks the record as cancelled. The SendGrid DELETE API call is also omitted in this path.", + "severity": "High" + } + ] + }, + { + "pr_title": "Advanced date override handling and timezone compatibility improvements", + "url": "https://github.com/calcom/cal.com/pull/8330", + "comments": [ + { + "comment": "Incorrect end time calculation using slotStartTime instead of slotEndTime", + "severity": "Medium" + }, + { + "comment": "Using === for dayjs object comparison will always return false as it compares object references, not values. Use .isSame() method instead: dayjs(date.start).add(utcOffset, 'minutes').isSame(dayjs(date.end).add(utcOffset, minutes))", + "severity": "Medium" + } + ] + }, + { + "pr_title": "OAuth credential sync and app integration enhancements", + "url": "https://github.com/calcom/cal.com/pull/11059", + "comments": [ + { + "comment": "The parseRefreshTokenResponse function incorrectly sets refresh_token to the hardcoded string 'refresh_token' when it's missing from the OAuth refresh token response. This invalidates the token, breaking subsequent token refreshes and causing authentication failures.", + "severity": "High" + }, + { + "comment": "Invalid Zod schema syntax. Computed property keys like [z.string().toString()] are not valid in Zod object schemas and will cause runtime errors. ", + "severity": "High" + }, + { + "comment": "parseRefreshTokenResponse returns a Zod safeParse result ({ success, data, error }), not the credential key object. Persisting that as key stores the wrapper instead of the token payload; we should store the parsed data or use schema parse.", + "severity": "High" + }, + { + "comment": "When APP_CREDENTIAL_SHARING_ENABLED and CALCOM_CREDENTIAL_SYNC_ENDPOINT are set, the refreshFunction helper returns the fetch Response, but several callers (for example GoogleCalendarService.refreshAccessToken expecting res.data, and HubspotCalendarService.refreshAccessToken expecting a HubspotToken) assume it returns the integration-specific token object. That mismatch will cause runtime errors in the sync-enabled path unless the return type or those call sites are adjusted.", + "severity": "High" + }, + { + "comment": "When the sync endpoint path is used, res is a fetch Response and has no .data; res?.data will be undefined and token.access_token will throw at runtime. This relies on a consistent return shape from refreshOAuthTokens, which isn’t guaranteed currently.", + "severity": "High" + } + ] + }, + { + "pr_title": "SMS workflow reminder retry count tracking", + "url": "https://github.com/calcom/cal.com/pull/14943", + "comments": [ + { + "comment": "Using retryCount: reminder.retryCount + 1 reads a possibly stale value and can lose increments under concurrency; consider an atomic increment via Prisma (increment: 1) to avoid race conditions (also applies to the similar update in the catch block).", + "severity": "High" + }, + { + "comment": "The deletion logic in scheduleSMSReminders.ts incorrectly deletes non-SMS workflow reminders (e.g., Email, WhatsApp) that have retryCount > 1. This occurs because the retryCount condition within the OR clause for deletion lacks a method: WorkflowMethods.SMS filter, causing it to apply to all reminder types instead of only SMS reminders, which is the intended scope of this function.", + "severity": "High" + } + ] + }, + { + "pr_title": "Add guest management functionality to existing bookings", + "url": "https://github.com/calcom/cal.com/pull/14740", + "comments": [ + { + "comment": "Case sensitivity bypass in email blacklist", + "severity": "High" + }, + { + "comment": "The logic for checking team admin/owner permissions is incorrect. This condition uses AND (&&) which requires both isTeamAdmin AND isTeamOwner to be true, but it should use OR (||) since a user needs to be either an admin OR an owner to have permission.", + "severity": "Critical" + }, + { + "comment": "This calls the email sender with the original guests, so existing attendees included in the input will be treated as new when sending notifications, leading to incorrect emails.", + "severity": "Medium" + }, + { + "comment": "uniqueGuests filters out existing attendees and blacklisted emails but does not deduplicate duplicates within the input; createMany can insert duplicate attendee rows if the client submits repeated emails.", + "severity": "Medium" + }, + { + "comment": "Starting with an array containing an empty string may cause validation issues. Consider starting with an empty array [] and handling the empty state in the MultiEmail component instead.", + "severity": "Low" + } + ] + }, + { + "pr_title": "feat: add calendar cache status and actions (#22532)", + "url": "https://github.com/calcom/cal.com/pull/22532", + "comments": [ + { + "comment": "The updateManyByCredentialId call uses an empty data object, which prevents Prisma's @updatedAt decorator from updating the updatedAt timestamp. This results in inaccurate cache status tracking, as the timestamp isn't updated when the cache is refreshed. To fix this, explicitly set the updatedAt field.", + "severity": "Medium" + }, + { + "comment": "logic: macOS-specific sed syntax with empty string after -i flag will fail on Linux systems", + "severity": "Low" + } + ] + } +] diff --git a/benchmarks/martian/golden_comments/discourse.json b/benchmarks/martian/golden_comments/discourse.json new file mode 100644 index 0000000..f1bbfd1 --- /dev/null +++ b/benchmarks/martian/golden_comments/discourse.json @@ -0,0 +1,184 @@ +[ + { + "pr_title": "FEATURE: automatically downsize large images", + "url": "https://github.com/ai-code-review-evaluation/discourse-graphite/pull/1", + "original_url": "https://github.com/discourse/discourse/commit/ffbaf8c54269df2ce510de91245760fddce09896", + "comments": [ + { + "comment": "The downsize method is defined twice. The second definition, which expects a single dimensions string parameter, overrides the first, which expected separate max_width and max_height parameters. This makes the original method unreachable and breaks existing code that calls it with separate width and height arguments.", + "severity": "Medium" + }, + { + "comment": "Hardcoding maxSizeKB = 10 * 1024 ignores Discourse.SiteSettings['max_' + type + '_size_kb'], so the client-side limit can diverge from server-side and per-type settings (also applies to the 413 handler below).", + "severity": "Low" + }, + { + "comment": "Passing 80% as the dimensions can fail for animated GIFs when allow_animated_thumbnails is true, since the animated path uses gifsicle --resize-fit which expects WxH geometry, not a percentage; downsizing would then silently fail.", + "severity": "Medium" + } + ] + }, + { + "pr_title": "FEATURE: per-topic unsubscribe option in emails", + "url": "https://github.com/ai-code-review-evaluation/discourse-graphite/pull/2", + "original_url": "https://github.com/discourse/discourse/commit/6669a2d94d76eea3b99b8c476d12b1eb66726b07", + "comments": [ + { + "comment": "logic: Potential nil pointer exception - if no TopicUser record exists, tu will be nil and calling methods on it will crash", + "severity": "High" + }, + { + "comment": "Typo in property name: 'stopNotificiationsText' should be 'stopNotificationsText' (missing 'n' in 'Notifications')", + "severity": "Low" + } + ] + }, + { + "pr_title": "Add comprehensive email validation for blocked users", + "url": "https://github.com/ai-code-review-evaluation/discourse-graphite/pull/3", + "original_url": "https://github.com/discourse/discourse/commit/5f8a130277dbddc95d133cd2832be639baf89213", + "comments": [ + { + "comment": "BlockedEmail.should_block_email? method has side effects during a read operation - it updates statistics even when just checking if an email should be blocked. This could cause race conditions in concurrent environments and makes the method name misleading.", + "severity": "Medium" + }, + { + "comment": "Regex pattern @(#{domains}) only matches domain suffixes, not full domains. evil.example.com would match whitelist entry example.com.", + "severity": "Medium" + } + ] + }, + { + "pr_title": "Enhance embed URL handling and validation system", + "url": "https://github.com/ai-code-review-evaluation/discourse-graphite/pull/4", + "original_url": "https://github.com/discourse/discourse/commit/4f8aed295a29954023b2849c060ef4fb299d1b5d", + "comments": [ + { + "comment": "SSRF vulnerability using open(url) without validation", + "severity": "Critical" + }, + { + "comment": "The current origin validation using indexOf is insufficient and can be bypassed. An attacker could use a malicious domain like evil-discourseUrl.com to pass this check.", + "severity": "Medium" + }, + { + "comment": "postMessage targetOrigin should be the origin (scheme+host+port), not the full referrer URL; using the full URL will cause the message to be dropped and prevent resizing.", + "severity": "Medium" + }, + { + "comment": "The code sets X-Frame-Options: ALLOWALL which completely disables clickjacking protection. The referer validation can be bypassed (referer headers are easily spoofed), and the fallback to empty string for nil referer masks validation failures.", + "severity": "Medium" + }, + { + "comment": "The TopicEmbed.import method is susceptible to a NoMethodError if the contents parameter is nil when attempting to append a string, and an XSS vulnerability due to unescaped url interpolation in the generated HTML.", + "severity": "Medium" + }, + { + "comment": "The ERB block closes with end if, which is invalid Ruby/ERB and will raise at render; it should just be end to close the if block.", + "severity": "Medium" + } + ] + }, + { + "pr_title": "Optimize header layout performance with flexbox mixins", + "url": "https://github.com/ai-code-review-evaluation/discourse-graphite/pull/5", + "original_url": "https://github.com/discourse/discourse/commit/5b229316ee4c661836ed1161139692a3e8527444", + "comments": [ + { + "comment": "Mixing float: left with flexbox causes layout issues. Further this PR removes the float-based right alignment for .d-header .panel, which may cause the login panel in the non-Ember/noscript header (where .panel is nested inside .row and not a flex item) to stack under the title instead of remaining right-aligned.", + "severity": "Low" + }, + { + "comment": "-ms-align-items never existed in any version of IE/Edge; the correct legacy property is -ms-flex-align.", + "severity": "Low" + } + ] + }, + { + "pr_title": "UX: show complete URL path if website domain is same as instance domain", + "url": "https://github.com/ai-code-review-evaluation/discourse-graphite/pull/6", + "original_url": "https://github.com/discourse/discourse/commit/267d8be1f556ed59639ced396c885bb44586da19", + "comments": [ + { + "comment": "The include_website_name method is missing the required ? suffix. Rails serializers expect include_ methods to end with ? for conditional attribute inclusion, a convention followed by other methods in this serializer. Without it, the website_name attribute may not be conditionally included as intended. Additionally, the '.' << website_host string concatenation should be replaced with '.' + website_host or '.#{website_host}' to avoid mutating string literals, which can lead to issues.", + "severity": "Medium" + } + ] + }, + { + "pr_title": "scale-color $lightness must use $secondary for dark themes", + "url": "https://github.com/ai-code-review-evaluation/discourse-graphite/pull/7", + "original_url": "https://github.com/discourse/discourse/commit/d38c4d5f7443223c81529c22470293771baf9f38", + "comments": [ + { + "comment": "In .topic-meta-data h5 a, the original code had color: scale-color($primary, $lightness: 30%) but was changed to dark-light-choose(scale-color($primary, $lightness: 70%), scale-color($secondary, $lightness: 30%)). The lightness for the light theme changed from 30% to 70%, which is a dramatic inversion", + "severity": "Low" + }, + { + "comment": "This change for desktop/user.css changes $primary from 30% to 50% for the light theme; most other changes preserve the original $primary value and move the complement to $secondary for dark. Consider reviewing this (also applies to a similar .name change in the mobile variant).", + "severity": "Low" + }, + { + "comment": "In topic-post.css the original code used $lightness: 70% but the replacement uses $lightness: 30% for the light theme. This makes the text significantly darker than intended.", + "severity": "Low" + } + ] + }, + { + "pr_title": "FIX: proper handling of group memberships", + "url": "https://github.com/ai-code-review-evaluation/discourse-graphite/pull/8", + "original_url": "https://github.com/discourse/discourse/commit/060cda77729cb1c4a827560e09e89a7b22078ba9", + "comments": [ + { + "comment": " The findMembers() call is now asynchronous and unhandled. The controller may not have member data immediately available, creating a race condition.", + "severity": "High" + }, + { + "comment": "In the next action, capping the next offset at user_count can produce an empty page (e.g., total equal to limit results in offset == total, showing 2/2 with no members). This can cause confusing UX on the last page.", + "severity": "Medium" + }, + { + "comment": "HTTP method mismatch in .remove_member - test uses PUT but remove_member action expects DELETE", + "severity": "Medium" + } + ] + }, + { + "pr_title": "FEATURE: Localization fallbacks (server-side)", + "url": "https://github.com/ai-code-review-evaluation/discourse-graphite/pull/9", + "original_url": "https://github.com/discourse/discourse/commit/ecfa17b5a79dfdc91e7a4d50b42ae78a35d0a293", + "comments": [ + { + "comment": "Thread-safety issue with lazy @loaded_locales", + "severity": "Low" + }, + { + "comment": "Consider normalizing the input locale (e.g., to a symbol) when checking/loading here to avoid double-loading if the same locale is passed as a String vs Symbol (also applies to other locations in the PR).", + "severity": "Low" + } + ] + }, + { + "pr_title": "FEATURE: Can edit category/host relationships for embedding", + "url": "https://github.com/ai-code-review-evaluation/discourse-graphite/pull/10", + "original_url": "https://github.com/discourse/discourse/commit/d1c69189f3c90ecf56013a8da904da9bff9a8e19", + "comments": [ + { + "comment": "NoMethodError before_validation in EmbeddableHost", + "severity": "Critical" + }, + { + "comment": "The update and destroy methods in Admin::EmbeddableHostsController do not validate the existence of the EmbeddableHost record retrieved by ID. If EmbeddableHost.where(id: params[:id]).first returns nil (i.e., the host does not exist), attempting to call methods on the nil object (e.g., save_host or destroy) will result in a NoMethodError.", + "severity": "Medium" + }, + { + "comment": "record_for_host compares lower(host) = ? but does not normalize the parameter’s case, so mixed‑case referer hosts may fail to match even though comparison intends to be case‑insensitive.", + "severity": "Medium" + }, + { + "comment": "Because this migration inserts embeddable_hosts rows with raw SQL, any existing embeddable_hosts values that include http:// or /https:// or path segments won’t go through the EmbeddableHost model’s normalization, so the new host lookup (which compares only the bare host) may fail for migrated data. Consider ensuring that migrated hosts are normalized to the same format as newly created EmbeddableHost records so existing embedding configurations keep working.", + "severity": "High" + } + ] + } +] diff --git a/benchmarks/martian/golden_comments/grafana.json b/benchmarks/martian/golden_comments/grafana.json new file mode 100644 index 0000000..1bd4862 --- /dev/null +++ b/benchmarks/martian/golden_comments/grafana.json @@ -0,0 +1,150 @@ +[ + { + "pr_title": "Anonymous: Add configurable device limit", + "url": "https://github.com/grafana/grafana/pull/79265", + "comments": [ + { + "comment": "Race condition: Multiple concurrent requests could pass the device count check simultaneously and create devices beyond the limit. Consider using a database transaction or lock.", + "severity": "High" + }, + { + "comment": "Anonymous authentication now fails entirely if anonDeviceService.TagDevice returns ErrDeviceLimitReached. Previously, device tagging was asynchronous and non-blocking. This change prevents anonymous users from authenticating when the device limit is reached.", + "severity": "Medium" + }, + { + "comment": "This call won’t compile: dbSession.Exec(args...) is given a []interface{} where the first element is the query, but Exec’s signature requires a first parameter of type string (not an interface{} splat).", + "severity": "Medium" + }, + { + "comment": "Returning ErrDeviceLimitReached when no rows were updated is misleading; the device might not exist.", + "severity": "Low" + }, + { + "comment": "Time window calculation inconsistency: Using device.UpdatedAt.UTC().Add(-anonymousDeviceExpiration) as the lower bound but device.UpdatedAt as the current time may not match the intended logic. Consider using time.Now().UTC() consistently.", + "severity": "Low" + } + ] + }, + { + "pr_title": "AuthZService: improve authz caching", + "url": "https://github.com/grafana/grafana/pull/103633", + "comments": [ + { + "comment": "The Check operation exhibits asymmetric cache trust logic: cached permission grants are trusted and returned immediately, but cached denials from the same permission cache are ignored, leading to a fresh database lookup. This allows stale cached grants to provide access to revoked resources, posing a security risk. ", + "severity": "High" + }, + { + "comment": "The test comment says the cached permissions 'allow access', but the map stores false for dashboards:uid:dash1, so checkPermission will still treat this scope as not allowed.", + "severity": "Low" + } + ] + }, + { + "pr_title": "Plugins: Chore: Renamed instrumentation middleware to metrics middleware", + "url": "https://github.com/grafana/grafana/pull/76186", + "comments": [ + { + "comment": "The ContextualLoggerMiddleware methods (QueryData, CallResource, CheckHealth, CollectMetrics) panic when a nil request is received. This occurs because they directly access req.PluginContext (via the instrumentContext function) without first checking if req is nil. This is a regression, as previous middleware layers gracefully handled nil requests.", + "severity": "High" + }, + { + "comment": "The traceID is no longer logged for plugin requests. During a refactoring, the tracing import and the logic to extract and add traceID from the context to log parameters were removed from the LoggerMiddleware. The newly introduced ContextualLoggerMiddleware does not add this information, resulting in missing traceID in plugin request logs and impacting debugging and request tracing capabilities.", + "severity": "Low" + } + ] + }, + { + "pr_title": "Advanced Query Processing Architecture", + "url": "https://github.com/grafana/grafana/pull/107534", + "comments": [ + { + "comment": "The applyTemplateVariables method is called with request.filters as the third parameter, but this parameter is not used in the corresponding test setup.", + "severity": "Low" + } + ] + }, + { + "pr_title": "Notification Rule Processing Engine", + "url": "https://github.com/grafana/grafana/pull/106778", + "comments": [ + { + "comment": "The rendered GrafanaRuleListItem is missing the required key prop for React list items. This can cause rendering issues when the list order changes.", + "severity": "Medium" + }, + { + "comment": "RuleActionsButtons is invoked with only promRule, but SilenceGrafanaRuleDrawer inside RuleActionsButtons still depends on a Grafana Ruler rule being present, so for Grafana rules coming from list views the 'Silence notifications' menu entry (now driven by Grafana Prom abilities) will toggle showSilenceDrawer without ever rendering the drawer. This means clicking 'Silence notifications' for these rules has no visible effect, even when abilities indicate silencing is allowed.", + "severity": "High" + } + ] + }, + { + "pr_title": "Dual Storage Architecture", + "url": "https://github.com/grafana/grafana/pull/90045", + "comments": [ + { + "comment": "The context is being created with d.Log instead of the log variable that was initialized with additional context values (name, kind, method). This means those values won't be propagated to the logging context.", + "severity": "Medium" + }, + { + "comment": "Bug: calling recordLegacyDuration when storage operation fails should be recordStorageDuration.", + "severity": "High" + }, + { + "comment": "Inconsistency: using name instead of options.Kind for metrics recording differs from other methods.", + "severity": "Medium" + } + ] + }, + { + "pr_title": "Database Performance Optimizations", + "url": "https://github.com/grafana/grafana/pull/80329", + "comments": [ + { + "comment": "The code uses Error log level for what appears to be debugging information. This will pollute error logs in production. Consider using Debug or Info level instead.", + "severity": "Low" + } + ] + }, + { + "pr_title": "Frontend Asset Optimization", + "url": "https://github.com/grafana/grafana/pull/90939", + "comments": [ + { + "comment": "The GetWebAssets function implements an incomplete double-checked locking pattern for caching web assets. The function first checks if the cache is populated using a read lock (RLock), and if the cache is empty, it acquires a write lock to populate it. However, it fails to re-check whether the cache was populated by another goroutine while waiting to acquire the write lock.", + "severity": "Medium" + }, + { + "comment": "In addition to the missing double-check, the function has a critical flaw in its error handling: it unconditionally assigns the fetch result to the cache (line 69: entryPointAssetsCache = result) regardless of whether the fetch succeeded or failed. When an error occurs during asset fetching, result is nil, and this nil value overwrites any previously valid cache entry.", + "severity": "High" + } + ] + }, + { + "pr_title": "Advanced SQL Analytics Framework", + "url": "https://github.com/grafana/grafana/pull/94942", + "comments": [ + { + "comment": "The enableSqlExpressions function has flawed logic that always returns false, effectively disabling SQL expressions unconditionally:", + "severity": "Critical" + }, + { + "comment": "Several methods such as NewInMemoryDB().RunCommands and db.QueryFramesInto return 'not implemented'.", + "severity": "High" + } + ] + }, + { + "pr_title": "Unified Storage Performance Optimizations", + "url": "https://github.com/grafana/grafana/pull/97529", + "comments": [ + { + "comment": "A race condition in BuildIndex allows multiple goroutines to concurrently build the same expensive index for the same key. This is caused by moving the b.cacheMu lock from protecting the entire function to only protecting the final cache assignment. ", + "severity": "High" + }, + { + "comment": "Calling s.search.TotalDocs() here may race with concurrent index creation: TotalDocs iterates b.cache without synchronization, and the event watcher goroutine started just above could trigger BuildIndex writes concurrently, potentially causing a concurrent map read/write panic.", + "severity": "High" + } + ] + } +] diff --git a/benchmarks/martian/golden_comments/keycloak.json b/benchmarks/martian/golden_comments/keycloak.json new file mode 100644 index 0000000..77c9d61 --- /dev/null +++ b/benchmarks/martian/golden_comments/keycloak.json @@ -0,0 +1,160 @@ +[ + { + "pr_title": "Fixing Re-authentication with passkeys", + "url": "https://github.com/ai-code-review-evaluation/keycloak-greptile/pull/1", + "original_url": "https://github.com/keycloak/keycloak/pull/41249", + "az_comment": "reviewed commit is not in the repo", + "comments": [ + { + "comment": "ConditionalPasskeysEnabled() called without UserModel parameter", + "severity": "Medium" + }, + { + "comment": "With isConditionalPasskeysEnabled(UserModel user) requiring user != null, authenticate(...) will not call webauthnAuth.fillContextForm(context) on the initial login page where context.getUser() is still null, so conditional passkey UI will not be set up for first-time passkey login. Consider whether this should also be enabled when no user has been selected yet so ID-less passkey authentication on the initial login form continues to work.", + "severity": "Medium" + } + ] + }, + { + "pr_title": "Add caching support for IdentityProviderStorageProvider.getForLogin operations", + "url": "https://github.com/keycloak/keycloak/pull/32918", + "comments": [ + { + "comment": "Recursive caching call using session instead of delegate", + "severity": "Critical" + }, + { + "comment": "Cleanup reference uses incorrect alias - should be 'idp-alias-' + i instead of 'alias'.", + "severity": "Medium" + } + ] + }, + { + "pr_title": "Add AuthzClientCryptoProvider for authorization client cryptographic operations", + "url": "https://github.com/keycloak/keycloak/pull/33832", + "comments": [ + { + "comment": "Returns wrong provider (default keystore instead of BouncyCastle)", + "severity": "High" + }, + { + "comment": "Dead code exists where ASN1Encoder instances are created and written to, but their results are immediately discarded. The actual encoding is performed by new ASN1Encoder instances created in the subsequent return statement, rendering the earlier operations useless.", + "severity": "Low" + } + ] + }, + { + "pr_title": "Add rolling-updates feature flag and compatibility framework", + "url": "https://github.com/keycloak/keycloak/pull/36882", + "comments": [ + { + "comment": "Incorrect method call for exit codes. The picocli.exit() method calls System.exit() directly, which is problematic:", + "severity": "Medium" + } + ] + }, + { + "pr_title": "Add Client resource type and scopes to authorization schema", + "url": "https://github.com/keycloak/keycloak/pull/36880", + "comments": [ + { + "comment": "Inconsistent feature flag bug causing orphaned permissions. The AdminPermissions event listener, responsible for cleaning up permissions upon role, client, or group removal, is incorrectly guarded by the ADMIN_FINE_GRAINED_AUTHZ (V1) feature flag. This is inconsistent with other methods in the class that use ADMIN_FINE_GRAINED_AUTHZ_V2. Consequently, if ADMIN_FINE_GRAINED_AUTHZ_V2 is enabled but V1 is not, the permission cleanup logic will not execute, leading to orphaned permission data. Cleanup should occur regardless of which fine-grained authorization version is enabled.", + "severity": "High" + }, + { + "comment": "In hasPermission(ClientModel client, String scope), the resource lookup uses findByName(server, client.getId(), server.getId()), but AdminPermissionsSchema.getOrCreateResource creates per-client resources with the owner set to resourceServer.getClientId(), so this lookup will never find those resources and will always fall back to the 'all-clients' resource, effectively ignoring client-specific permissions.", + "severity": "High" + }, + { + "comment": "In getClientsWithPermission(String scope), iterating resourceStore.findByType(server, AdminPermissionsSchema.CLIENTS_RESOURCE_TYPE) and returning resource.getName() will only ever consider the type-level 'Clients' resource (per-client resources have no type) and return its name, while AvailableRoleMappingResource#getRoleIdsWithPermissions expects actual client IDs to pass to realm.getClientById, which can lead to incorrect behavior or a null client and subsequent failures.", + "severity": "High" + } + ] + }, + { + "pr_title": "Add Groups resource type and scopes to authorization schema", + "url": "https://github.com/keycloak/keycloak/pull/37038", + "comments": [ + { + "comment": "Incorrect permission check in canManage() method", + "severity": "High" + }, + { + "comment": "In getGroupIdsWithViewPermission, hasPermission is called with groupResource.getId() and the same groupResource.getId() is added to granted, but hasPermission resolves resources by name (treating the argument as a group id) and the GroupPermissionEvaluator contract says this method returns group IDs that are later used as UserModel.GROUPS and in getUsersCount group filters. This mismatch means per-group VIEW_MEMBERS/MANAGE_MEMBERS permissions may not yield the expected group IDs for filtering and counts, and evaluation may effectively only look at the type-level 'all-groups' resource; consider revisiting whether this should operate on the underlying group ids (resource names) instead so it aligns with the JPA queries and the interface contract.", + "severity": "High" + } + ] + }, + { + "pr_title": "Add HTML sanitizer for translated message resources", + "url": "https://github.com/keycloak/keycloak/pull/37429", + "comments": [ + { + "comment": "The translation is in Italian instead of Lithuanian. This should be translated to Lithuanian to match the file's locale (messages_lt.properties).", + "severity": "Medium" + }, + { + "comment": "The totpStep1 value uses Traditional Chinese terms in the Simplified Chinese file (zh_CN), which is likely incorrect for this locale. Please verify the locale‑appropriate translation.", + "severity": "Medium" + }, + { + "comment": "The anchor sanitization logic has a potential issue where it consumes English matcher groups without proper validation. If the translated text has more anchor tags than the English text, this could lead to incorrect validation results.", + "severity": "Low" + }, + { + "comment": "The method name 'santizeAnchors' should be 'sanitizeAnchors' (missing 'i').", + "severity": "Low" + } + ] + }, + { + "pr_title": "Implement access token context encoding framework", + "url": "https://github.com/keycloak/keycloak/pull/37634", + "comments": [ + { + "comment": "Wrong parameter in null check (grantType vs. rawTokenId)", + "severity": "Critical" + }, + { + "comment": "In isAccessTokenId, the substring for the grant shortcut and the equality check look inverted: the grant shortcut occupies indices 4–5 (substring(4,6)), and a match should return true (combined with UUID check), not false.", + "severity": "High" + }, + { + "comment": "Javadoc mentions \"usually like 3-letters shortcut\" but some implementations use 2-letter shortcuts (\"ac\", \"cc\", \"rt\", \"te\", \"pc\", \"ci\", \"ro\"). Consider updating documentation to reflect actual usage pattern.", + "severity": "Low" + }, + { + "comment": " Catching generic RuntimeException is too broad. The implementation throws IllegalArgumentException specifically - catch that instead for more precise testing.", + "severity": "Low" + } + ] + }, + { + "pr_title": "Implement recovery key support for user storage providers", + "url": "https://github.com/keycloak/keycloak/pull/38446", + "comments": [ + { + "comment": "Unsafe raw List deserialization without type safety. Calling Optional.get() directly on the Optional returned by RecoveryAuthnCodesUtils.getCredential(user) without checking isPresent() can lead to a NoSuchElementException if the Optional is empty.", + "severity": "Medium" + }, + { + "comment": "After creating the RecoveryAuthnCodesCredentialModel, consider setting its id from the stored credential (e.g., myUser.recoveryCodes.getId()); otherwise getId() will be null and downstream removal by id (e.g., removeStoredCredentialById in the authenticator flow) may not work.", + "severity": "Low" + } + ] + }, + { + "pr_title": "Fix concurrent group access to prevent NullPointerException", + "url": "https://github.com/keycloak/keycloak/pull/40940", + "comments": [ + { + "comment": "Returning null from getSubGroupsCount() violates the GroupModel contract (Javadoc says it never returns null) and may lead to NPEs in callers that expect a non-null count.", + "severity": "Critical" + }, + { + "comment": "The reader thread isn’t waited for; flipping deletedAll to true and asserting immediately can race and miss exceptions added just after the flag change, making this test flaky.", + "severity": "Medium" + } + ] + } +] diff --git a/benchmarks/martian/golden_comments/sentry.json b/benchmarks/martian/golden_comments/sentry.json new file mode 100644 index 0000000..78f186e --- /dev/null +++ b/benchmarks/martian/golden_comments/sentry.json @@ -0,0 +1,196 @@ +[ + { + "pr_title": "Enhanced Pagination Performance for High-Volume Audit Logs", + "url": "https://github.com/ai-code-review-evaluation/sentry-greptile/pull/1", + "az_comment": "reviewed commit is not in the repo", + "comments": [ + { + "comment": "Importing non-existent OptimizedCursorPaginator", + "severity": "Low" + }, + { + "comment": "Django querysets do not support negative slicing", + "severity": "High" + }, + { + "comment": "When requests are authenticated with API keys or org auth tokens (which have user_id=None), organization_context.member is None. Line 71 attempts to access organization_context.member.has_global_access without checking if member is None, causing an AttributeError crash when optimized_pagination=true is used, even though the request passed all permission checks with valid org:write scope.", + "severity": "High" + }, + { + "comment": "get_item_key assumes a numeric key, but the paginator is used with order_by=-datetime in the audit logs endpoint; calling math.floor/ceil on a datetime will raise a TypeError.", + "severity": "High" + } + ] + }, + { + "pr_title": "Optimize spans buffer insertion with eviction during insert", + "url": "https://github.com/ai-code-review-evaluation/sentry-greptile/pull/2", + "original_url": "https://github.com/getsentry/sentry/pull/92393", + "az_comment": "reviewed commit is not in the repo", + "comments": [ + { + "comment": "OptimizedCursorPaginator negative-offset branch slices QuerySet with a negative start index", + "severity": "Critical" + }, + { + "comment": "BasePaginator negative-offset branch slices QuerySet with a negative start index", + "severity": "High" + }, + { + "comment": "OptimizedCursorPaginator.get_item_key uses floor/ceil on a datetime key (order_by='-datetime'), causing TypeError.", + "severity": "High" + } + ] + }, + { + "pr_title": "feat(upsampling) - Support upsampled error count with performance optimizations", + "url": "https://github.com/ai-code-review-evaluation/sentry-greptile/pull/3", + "original_url": "https://github.com/getsentry/sentry/pull/94376", + "az_comment": "reviewed commit is not in the repo", + "comments": [ + { + "comment": "sample_rate = 0.0 is falsy and skipped", + "severity": "Low" + }, + { + "comment": "Using Python’s built-in hash() to build cache keys is non-deterministic across processes (hash randomization), so keys won’t match across workers and invalidate_upsampling_cache may fail to delete them. Use a deterministic serialization of project_ids for the cache key.", + "severity": "Low" + }, + { + "comment": "The upsampling eligibility check passes the outer dataset instead of the actual dataset used by scoped_dataset. In paths where the query ultimately runs against discover (e.g., dashboard split) while the original dataset is metrics, upsampling may be skipped even when all projects are allowlisted.", + "severity": "Medium" + } + ] + }, + { + "pr_title": "GitHub OAuth Security Enhancement", + "url": "https://github.com/getsentry/sentry/pull/67876", + "comments": [ + { + "comment": "Null reference if github_authenticated_user state is missing", + "severity": "Medium" + }, + { + "comment": "OAuth state uses pipeline.signature (static) instead of a per-request random value", + "severity": "Medium" + }, + { + "comment": "The code attempts to access integration.metadata[sender][login] without checking for the existence of the sender key. This causes a KeyError for integrations where the sender metadata was not set during creation", + "severity": "High" + } + ] + }, + { + "pr_title": "Replays Self-Serve Bulk Delete System", + "url": "https://github.com/ai-code-review-evaluation/sentry-greptile/pull/5", + "az_comment": "there is no such PR, it is a mix of many PRs", + "comments": [ + { + "comment": "Breaking changes in error response format", + "severity": "Medium" + }, + { + "comment": "Detector validator uses wrong key when updating type", + "severity": "Medium" + }, + { + "comment": "Using zip(error_ids, events.values()) assumes the get_multi result preserves the input order; dict value order is not guaranteed to match error_ids, so event data can be paired with the wrong ID (missing nodes also shift alignment).", + "severity": "Low" + } + ] + }, + { + "pr_title": "Span Buffer Multiprocess Enhancement with Health Monitoring", + "url": "https://github.com/getsentry/sentry/pull/93824", + "comments": [ + { + "comment": "Inconsistent metric tagging with 'shard' and 'shards'", + "severity": "Medium" + }, + { + "comment": "Fixed sleep in tests can be flaky; wait on condition instead", + "severity": "Low" + }, + { + "comment": "Because flusher processes are created via multiprocessing.get_context('spawn').Process, they are instances of multiprocessing.context.SpawnProcess, which on POSIX is not a subclass of multiprocessing.Process, so this isinstance check will always be false and hung processes won't be killed here.", + "severity": "High" + }, + { + "comment": "Sleep in test_consumer.py won’t actually wait because time.sleep was monkeypatched above; consider restoring sleep or using a different sync to ensure the flusher has time to process.", + "severity": "Medium" + }, + { + "comment": "Breaking out of the loop when the deadline has elapsed can skip terminating remaining flusher processes, potentially leaving them running after shutdown; consider ensuring termination is attempted even if the deadline is exceeded.", + "severity": "Medium" + } + ] + }, + { + "pr_title": "feat(ecosystem): Implement cross-system issue synchronization", + "url": "https://github.com/getsentry/sentry/pull/77754", + "comments": [ + { + "comment": "Shared mutable default in dataclass timestamp", + "severity": "Medium" + }, + { + "comment": "The method name has a typo: test_from_dict_inalid_data should be test_from_dict_invalid_data.", + "severity": "Low" + }, + { + "comment": "Method name says 'empty_array' but tests empty dict - consider renaming to 'test_from_dict_empty_dict' for clarity.", + "severity": "Low" + }, + { + "comment": "to_dict() returns a datetime for queued; if this dict is passed in task kwargs (e.g., via apply_async), JSON serialization may fail depending on the serializer, which can cause enqueue errors.", + "severity": "Medium" + } + ] + }, + { + "pr_title": "ref(crons): Reorganize incident creation / issue occurrence logic", + "url": "https://github.com/getsentry/sentry/pull/80528", + "comments": [ + { + "comment": "The function modifies the config variable to include display values but then returns the original monitor.config instead of the modified version.", + "severity": "High" + }, + { + "comment": "The code fetches MonitorCheckIn objects by ID when the required data already exists in previous_checkins. This creates an unnecessary database query.", + "severity": "Low" + } + ] + }, + { + "pr_title": "feat(uptime): Add ability to use queues to manage parallelism", + "url": "https://github.com/getsentry/sentry/pull/95633", + "comments": [ + { + "comment": "The queue.shutdown() method with 'immediate=False' parameter may not exist in the standard Python queue module. This could cause AttributeError at runtime. Verify the correct API or implement a custom shutdown mechanism.", + "severity": "High" + }, + { + "comment": "The magic number 50 for max_wait is used repeatedly throughout the tests. Consider extracting this as a named constant to improve maintainability.", + "severity": "Low" + }, + { + "comment": "The test test_thread_queue_parallel_error_handling has a docstring that doesn't match the test implementation.", + "severity": "Low" + } + ] + }, + { + "pr_title": "feat(workflow_engine): Add in hook for producing occurrences from the stateful detector", + "url": "https://github.com/getsentry/sentry/pull/80168", + "comments": [ + { + "comment": "MetricAlertDetectorHandler inherits from StatefulDetectorHandler but only contains pass, failing to implement its required abstract methods: counter_names (property), get_dedupe_value(), get_group_key_values(), and build_occurrence_and_event_data(). This will cause a TypeError at runtime when the class is instantiated.", + "severity": "High" + }, + { + "comment": "Docstring says this returns a list of DetectorEvaluationResult, but the method now returns a dict keyed by DetectorGroupKey. Consider updating the docstring to match the new return type.", + "severity": "Low" + } + ] + } +] diff --git a/benchmarks/martian/judge.py b/benchmarks/martian/judge.py new file mode 100644 index 0000000..12df96c --- /dev/null +++ b/benchmarks/martian/judge.py @@ -0,0 +1,208 @@ +# SPDX-License-Identifier: MIT +"""Step 4: Judge candidates against golden comments. + +Uses Martian's judge prompt verbatim. Greedy best-confidence matching. +Prompt vendored from Martian @ 012d682. +""" + +from __future__ import annotations + +import json +from collections.abc import Callable +from pathlib import Path + +from benchmarks.martian.config import BenchConfig +from benchmarks.martian.fetch_diffs import parse_golden_prs + +# --- Vendored Martian prompt (loaded from file, not inline) --- +_PROMPTS_DIR = Path(__file__).resolve().parent / "prompts" + +JUDGE_PROMPT = (_PROMPTS_DIR / "judge.txt").read_text().strip() + +# --- End vendored prompt --- + + +def judge_pair( + golden_comment: str, + candidate_text: str, + *, + judge_fn: Callable[[str, str], dict] | None = None, +) -> dict: + """Judge whether a candidate matches a golden comment. + + Returns: {reasoning, match, confidence} + """ + if judge_fn is not None: + return judge_fn(golden_comment, candidate_text) + return _llm_judge(golden_comment, candidate_text) + + +def _llm_judge(golden_comment: str, candidate_text: str) -> dict: + """Default judge using Claude.""" + import anthropic + + client = anthropic.Anthropic() + resp = client.messages.create( + model="claude-sonnet-4-5-20250929", + max_tokens=512, + temperature=0.0, + messages=[ + { + "role": "user", + "content": JUDGE_PROMPT.format( + golden_comment=golden_comment, + candidate=candidate_text, + ), + } + ], + ) + content = resp.content[0].text.strip() + # Strip markdown code fences if present (Claude sometimes wraps JSON) + if content.startswith("```"): + content = content.split("```")[1] + if content.startswith("json"): + content = content[4:] + content = content.strip() + return json.loads(content) + + +def match_candidates_to_golden( + verdicts: list[dict], + n_golden: int, + n_candidates: int, +) -> dict[int, int | None]: + """Greedy assignment: each golden matches its highest-confidence candidate. + + Each candidate can only be assigned to one golden (first-come). + Returns: {golden_idx: candidate_idx or None} + """ + # Sort by confidence descending + matches_by_golden: dict[int, list] = {i: [] for i in range(n_golden)} + for v in verdicts: + if v["match"]: + matches_by_golden[v["golden_idx"]].append(v) + + # Sort each golden's matches by confidence + for g in matches_by_golden: + matches_by_golden[g].sort(key=lambda x: x["confidence"], reverse=True) + + # Greedy assignment + assigned_candidates: set[int] = set() + result: dict[int, int | None] = {} + + # Process goldens by their best available confidence + golden_order = sorted( + range(n_golden), + key=lambda g: matches_by_golden[g][0]["confidence"] if matches_by_golden[g] else -1, + reverse=True, + ) + + for g in golden_order: + result[g] = None + for v in matches_by_golden[g]: + if v["candidate_idx"] not in assigned_candidates: + result[g] = v["candidate_idx"] + assigned_candidates.add(v["candidate_idx"]) + break + + return result + + +def compute_metrics( + tp: int, + total_candidates: int, + total_golden: int, +) -> dict: + """Compute precision, recall, F1.""" + precision = tp / total_candidates if total_candidates > 0 else 0.0 + recall = tp / total_golden if total_golden > 0 else 0.0 + f1 = (2 * precision * recall / (precision + recall)) if (precision + recall) > 0 else 0.0 + return {"precision": precision, "recall": recall, "f1": f1, "tp": tp} + + +def judge_all(config: BenchConfig | None = None) -> None: + """Judge all candidates against golden comments.""" + config = config or BenchConfig() + prs = parse_golden_prs(config.golden_dir) + cand_dir = config.output_dir / "candidates" + score_dir = config.output_dir / "scores" + score_dir.mkdir(parents=True, exist_ok=True) + + all_results = [] + total_tp = 0 + total_candidates = 0 + total_golden = 0 + + for pr in prs: + slug = f"{pr['repo']}_PR{pr['pr_number']}" + cand_path = cand_dir / f"{slug}.json" + + if not cand_path.exists(): + print(f"{slug}: no candidates — recording as missing") + all_results.append( + { + "pr": slug, + "golden_url": pr["golden_url"], + "n_candidates": 0, + "n_golden": len(pr["golden_comments"]), + "status": "missing_candidates", + "metrics": {"precision": 0.0, "recall": 0.0, "f1": 0.0, "tp": 0}, + } + ) + total_golden += len(pr["golden_comments"]) + continue + + candidates = json.loads(cand_path.read_text()) + golden = pr["golden_comments"] + + print(f"{slug}: {len(candidates)} candidates vs {len(golden)} golden") + + # All-pairs judging + verdicts = [] + for gi, g in enumerate(golden): + for ci, c in enumerate(candidates): + v = judge_pair(g["comment"], c["text"]) + v["golden_idx"] = gi + v["candidate_idx"] = ci + verdicts.append(v) + + # Greedy matching + matches = match_candidates_to_golden(verdicts, len(golden), len(candidates)) + tp = sum(1 for v in matches.values() if v is not None) + + metrics = compute_metrics(tp, len(candidates), len(golden)) + print(f" → P={metrics['precision']:.2f} R={metrics['recall']:.2f} F1={metrics['f1']:.2f}") + + all_results.append( + { + "pr": slug, + "golden_url": pr["golden_url"], + "n_candidates": len(candidates), + "n_golden": len(golden), + "verdicts": verdicts, + "matches": {str(k): v for k, v in matches.items()}, + "metrics": metrics, + } + ) + total_tp += tp + total_candidates += len(candidates) + total_golden += len(golden) + + # Aggregate + overall = compute_metrics(total_tp, total_candidates, total_golden) + output = { + "provenance": config.stamp(), + "overall": overall, + "per_pr": all_results, + } + + score_path = score_dir / "judge_results.json" + score_path.write_text(json.dumps(output, indent=2)) + print( + f"\nOverall: P={overall['precision']:.2f} R={overall['recall']:.2f} F1={overall['f1']:.2f}" + ) + print(f"Saved to {score_path}") + + +if __name__ == "__main__": + judge_all() diff --git a/benchmarks/martian/output/.gitkeep b/benchmarks/martian/output/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/benchmarks/martian/prompts/CHECKSUMS.md b/benchmarks/martian/prompts/CHECKSUMS.md new file mode 100644 index 0000000..710ea93 --- /dev/null +++ b/benchmarks/martian/prompts/CHECKSUMS.md @@ -0,0 +1,8 @@ +# Vendored Martian Prompts + +Source: withmartian/code-review-benchmark @ 012d68202e56280855b85abb956410086008a7b2 +- extract.txt + extract_system.txt from: offline/code_review_benchmark/step2_extract_comments.py +- judge.txt from: offline/code_review_benchmark/step3_judge_comments.py + +SHA-256 checksums in CHECKSUMS.sha256. +If prompts need updating, re-vendor from a new pinned commit — never edit in place. diff --git a/benchmarks/martian/prompts/CHECKSUMS.sha256 b/benchmarks/martian/prompts/CHECKSUMS.sha256 new file mode 100644 index 0000000..98e4925 --- /dev/null +++ b/benchmarks/martian/prompts/CHECKSUMS.sha256 @@ -0,0 +1,3 @@ +c2edd6726269a219040c86594f2d835ffe172fb9964af050cee098e4bc029c32 extract.txt +174f73455cc472366f024dc73f6aa641f058b7c8d968884e77f661e84c4e3772 extract_system.txt +43882bd12af0f8f41cd284ff1dc248919f06895e25cdcd7fd50d7a80c53214fc judge.txt diff --git a/benchmarks/martian/prompts/extract.txt b/benchmarks/martian/prompts/extract.txt new file mode 100644 index 0000000..a1d2c09 --- /dev/null +++ b/benchmarks/martian/prompts/extract.txt @@ -0,0 +1,28 @@ +You are analyzing an AI code review comment to extract individual issues mentioned. + +The comment may discuss multiple distinct problems. Extract each separate issue as a standalone item. + +Code Review Comment: +{comment} + +Instructions: +- Extract each distinct code issue, bug, or concern mentioned +- Each issue should be a single, specific problem (not a general observation) +- Ignore meta-commentary like "I found 2 issues" - extract the actual issues +- Ignore sign-offs, greetings, or formatting instructions +- If the comment contains no actionable code review issues, return an empty list + +Example input: +"Found several problems: 1) The getUserById function doesn't handle null input, which will cause a crash. +2) The cache key uses user.name but should use user.id for uniqueness. +Also, consider adding retry logic for the API call." + +Example output: +{{"issues": [ + "getUserById function doesn't handle null input, causing potential crash", + "Cache key uses user.name instead of user.id, breaking uniqueness", + "Missing retry logic for API call" +]}} + +Respond with ONLY a JSON object: +{{"issues": ["issue 1", "issue 2", ...]}} \ No newline at end of file diff --git a/benchmarks/martian/prompts/extract_system.txt b/benchmarks/martian/prompts/extract_system.txt new file mode 100644 index 0000000..fb1fb26 --- /dev/null +++ b/benchmarks/martian/prompts/extract_system.txt @@ -0,0 +1 @@ +You extract code review issues from comments. Always respond with valid JSON. \ No newline at end of file diff --git a/benchmarks/martian/prompts/judge.txt b/benchmarks/martian/prompts/judge.txt new file mode 100644 index 0000000..a24ac69 --- /dev/null +++ b/benchmarks/martian/prompts/judge.txt @@ -0,0 +1,16 @@ +You are evaluating AI code review tools. +Determine if the candidate issue matches the golden (expected) comment. + +Golden Comment (the issue we're looking for): +{golden_comment} + +Candidate Issue (from the tool's review): +{candidate} + +Instructions: +- Determine if the candidate identifies the SAME underlying issue as the golden comment +- Accept semantic matches - different wording is fine if it's the same problem +- Focus on whether they point to the same bug, concern, or code issue + +Respond with ONLY a JSON object: +{{"reasoning": "brief explanation", "match": true/false, "confidence": 0.0-1.0}} \ No newline at end of file diff --git a/benchmarks/martian/pyproject.toml b/benchmarks/martian/pyproject.toml new file mode 100644 index 0000000..5695cc3 --- /dev/null +++ b/benchmarks/martian/pyproject.toml @@ -0,0 +1,15 @@ +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[project] +name = "grippy-martian-bench" +version = "0.1.0" +description = "Benchmark harness: Grippy vs Martian code-review-benchmark" +license = "MIT" +requires-python = ">=3.12" +dependencies = [ + "grippy-mcp>=0.1.0", + "httpx>=0.27", + "anthropic>=0.40", +] diff --git a/benchmarks/martian/report.py b/benchmarks/martian/report.py new file mode 100644 index 0000000..f949f4e --- /dev/null +++ b/benchmarks/martian/report.py @@ -0,0 +1,136 @@ +# SPDX-License-Identifier: MIT +"""Step 5: Compute and display benchmark metrics.""" + +from __future__ import annotations + +import json +import sys +from urllib.parse import urlparse + +from benchmarks.martian.config import BenchConfig +from benchmarks.martian.judge import compute_metrics + + +def aggregate_by_repo(per_pr: list[dict]) -> dict: + """Aggregate metrics by source repository.""" + repos: dict[str, dict] = {} + for pr in per_pr: + url = pr.get("golden_url", "") + parts = urlparse(url).path.strip("/").split("/") + repo = parts[1] if len(parts) >= 2 else pr["pr"].split("_")[0] + + if repo not in repos: + repos[repo] = {"n_prs": 0, "tp": 0, "candidates": 0, "golden": 0} + + repos[repo]["n_prs"] += 1 + repos[repo]["tp"] += pr["metrics"]["tp"] + repos[repo]["candidates"] += pr["n_candidates"] + repos[repo]["golden"] += pr["n_golden"] + + for _repo, data in repos.items(): + m = compute_metrics(data["tp"], data["candidates"], data["golden"]) + data.update(m) + + return repos + + +def format_table(results: dict) -> list[str]: + """Format results as a text table.""" + lines = [] + overall = results["overall"] + + lines.append("=" * 60) + lines.append("GRIPPY BENCHMARK RESULTS") + lines.append("=" * 60) + lines.append("") + lines.append(f" Precision: {overall['precision']:.1%}") + lines.append(f" Recall: {overall['recall']:.1%}") + lines.append(f" F1: {overall['f1']:.1%}") + lines.append(f" TP: {overall['tp']}") + lines.append("") + + # Per-repo breakdown + by_repo = aggregate_by_repo(results["per_pr"]) + lines.append("-" * 60) + lines.append(f"{'Repo':<20} {'PRs':>4} {'P':>7} {'R':>7} {'F1':>7}") + lines.append("-" * 60) + for repo, data in sorted(by_repo.items()): + lines.append( + f"{repo:<20} {data['n_prs']:>4} " + f"{data['precision']:>6.1%} {data['recall']:>6.1%} {data['f1']:>6.1%}" + ) + lines.append("-" * 60) + + prov = results.get("provenance", {}) + if prov: + lines.append("") + lines.append(f"Model: {prov.get('model_id', '?')}") + lines.append(f"Profile: {prov.get('profile', '?')}") + lines.append(f"Martian: {prov.get('martian_commit', '?')[:8]}") + + return lines + + +def report(config: BenchConfig | None = None) -> None: + """Print benchmark report from judge results. + + Surfaces skipped/failed PRs prominently — silent omission is how + people accuse you of laundering the score. + """ + config = config or BenchConfig() + score_path = config.output_dir / "scores" / "judge_results.json" + + if not score_path.exists(): + print(f"ERROR: No judge results at {score_path}", file=sys.stderr) + sys.exit(1) + + results = json.loads(score_path.read_text()) + for line in format_table(results): + print(line) + + # Unified failure accounting across ALL phases + unique_failures: dict[str, dict] = {} + for manifest_name in ("run_manifest.json", "extract_manifest.json"): + mp = config.output_dir / manifest_name + if mp.exists(): + entries = json.loads(mp.read_text()) + entries = entries.get("results", entries) if isinstance(entries, dict) else entries + for r in entries: + if r.get("status") == "failed": + pr_slug = r["pr"] + if pr_slug not in unique_failures: + unique_failures[pr_slug] = {**r, "phase": manifest_name.split("_")[0]} + + # Also check judge results for missing candidates + for pr_result in results.get("per_pr", []): + if pr_result.get("status") == "missing_candidates": + pr_slug = pr_result["pr"] + if pr_slug not in unique_failures: + unique_failures[pr_slug] = { + "pr": pr_slug, + "status": "missing_candidates", + "phase": "judge", + } + + failed = list(unique_failures.values()) + total = len(results.get("per_pr", [])) + + if failed: + print() + print("=" * 60) + print("FAILURE ACCOUNTING (all phases)") + print("=" * 60) + for f in failed: + phase = f.get("phase", "?") + reason = f.get("reason", f.get("status", "unknown")) + print(f" [{phase}] {f['pr']} — {reason}") + pct = len(failed) / total * 100 if total else 0 + print(f"\n {len(failed)}/{total} failed ({pct:.0f}%)") + if pct > 10: + print(" WARNING: >10% failure rate — run NOT valid for public claims") + else: + print(f"\n No failures across {total} PRs.") + + +if __name__ == "__main__": + report() diff --git a/benchmarks/martian/run_grippy.py b/benchmarks/martian/run_grippy.py new file mode 100644 index 0000000..60766aa --- /dev/null +++ b/benchmarks/martian/run_grippy.py @@ -0,0 +1,220 @@ +# SPDX-License-Identifier: MIT +"""Step 2: Run Grippy reviews against fetched diffs.""" + +from __future__ import annotations + +import json +import sys +from datetime import UTC, datetime + +from benchmarks.martian.config import BenchConfig +from benchmarks.martian.fetch_diffs import parse_golden_prs + + +def _format_rules_for_agent(rule_findings: list) -> str: + """Format rule findings as text for the agent's context. + + Own implementation — avoids coupling to grippy.review internals. + This is simple string formatting; the agent just needs to see the + rule results to avoid duplicating them. + """ + lines = [] + for r in rule_findings: + loc = f"{r.file}:{r.line}" if r.line else r.file + lines.append(f"[{r.severity.name}] {r.rule_id}: {r.message} ({loc})") + return "\n".join(lines) + + +def is_inline_finding(finding: dict) -> bool: + """True if finding has a specific file and line anchor.""" + return bool(finding.get("file")) and bool(finding.get("line_start")) + + +def format_finding_as_comment(finding: dict) -> str: + """Format a finding as a reviewer comment for benchmark extraction. + + Mirrors the structure of Grippy's production inline comments + (see github_review.py:build_review_comment) but strips metadata + that would bias the Martian judge: + - No severity emoji/tags (judge should match on issue, not label) + - No confidence score (internal metric) + - No suggestion (judge matches problems, not fixes) + - No grippy_note (personality, not substance) + - No dedup marker (internal bookkeeping) + + What remains: title + description, which is the issue substance + that a human reviewer would write. + """ + title = finding.get("title", "").strip() + desc = finding.get("description", "").strip() + # Markdown heading mirrors production format structure + return f"#### {title}\n\n{desc}" + + +def review_single_pr( + *, + diff_text: str, + pr_title: str, + config: BenchConfig, +) -> dict: + """Run Grippy review on a single diff. Returns serialized review dict. + + Runs deterministic rules with the frozen profile, then LLM review. + Profile is passed explicitly — never relies on ambient env for this. + """ + from grippy.agent import create_reviewer, format_pr_context + from grippy.retry import run_review + from grippy.rules import load_profile, run_rules + + # Deterministic rules — profile from frozen config, explicit + profile_cfg = load_profile(cli_profile=config.profile) + rule_findings = run_rules(diff_text, profile_cfg) + + # Format rule findings for the agent (only if profile enables them) + # Own formatter — avoids reaching into grippy.review internals + rule_text = "" + if rule_findings and config.profile != "general": + rule_text = _format_rules_for_agent(rule_findings) + + agent = create_reviewer( + model_id=config.model_id, + base_url=config.base_url, + api_key=config.api_key, + transport=config.transport, + mode=config.mode, + include_rule_findings=bool(rule_text), + ) + message = format_pr_context( + title=pr_title, + author="benchmark", + branch="feature → main", + diff=diff_text, + rule_findings=rule_text, + ) + review = run_review(agent, message) + return review.model_dump() + + +def run_all(config: BenchConfig | None = None, resume: bool = False) -> None: + """Run Grippy on all fetched diffs.""" + config = config or BenchConfig.from_env() + prs = parse_golden_prs(config.golden_dir) + + diff_dir = config.output_dir / "diffs" + review_dir = config.output_dir / "reviews" + comment_dir = config.output_dir / "comments" + review_dir.mkdir(parents=True, exist_ok=True) + comment_dir.mkdir(parents=True, exist_ok=True) + + manifest = [] + for i, pr in enumerate(prs, 1): + slug = f"{pr['repo']}_PR{pr['pr_number']}" + review_path = review_dir / f"{slug}.json" + comment_path = comment_dir / f"{slug}.json" + + if resume and review_path.exists() and comment_path.exists(): + print(f"[{i}/{len(prs)}] {slug} — skipped (resume, both review + comments exist)") + continue + if resume and review_path.exists() and not comment_path.exists(): + print(f"[{i}/{len(prs)}] {slug} — regenerating comments from cached review") + try: + review_data = json.loads(review_path.read_text()).get("review", {}) + findings = review_data.get("findings", []) + inline_comments = [] + general_comments = [] + for f in findings: + body = format_finding_as_comment(f) + if is_inline_finding(f): + inline_comments.append( + {"path": f["file"], "line": f["line_start"], "body": body} + ) + else: + general_comments.append(body) + comment_path.write_text( + json.dumps( + { + "inline": inline_comments, + "general": general_comments, + }, + indent=2, + ) + ) + manifest.append({"pr": slug, "status": "ok", "findings": len(findings)}) + except Exception as e: + print(f"[{i}/{len(prs)}] {slug} — ERROR regenerating: {e}", file=sys.stderr) + manifest.append({"pr": slug, "status": "failed", "reason": str(e)}) + continue + + diff_path = diff_dir / f"{slug}.diff" + if not diff_path.exists(): + print(f"[{i}/{len(prs)}] {slug} — ERROR: diff not found", file=sys.stderr) + manifest.append({"pr": slug, "status": "failed", "reason": "diff_not_found"}) + continue + + diff_text = diff_path.read_text() + print(f"[{i}/{len(prs)}] {slug} — reviewing ({len(diff_text)} chars)") + + ts = datetime.now(UTC).isoformat() + try: + review_data = review_single_pr( + diff_text=diff_text, + pr_title=pr["pr_title"], + config=config, + ) + + # Save full review + output = {"provenance": config.stamp(), "timestamp": ts, "review": review_data} + review_path.write_text(json.dumps(output, indent=2)) + + # Format findings as comments for extraction step + findings = review_data.get("findings", []) + inline_comments = [] + general_comments = [] + for f in findings: + body = format_finding_as_comment(f) + if is_inline_finding(f): + inline_comments.append( + { + "path": f["file"], + "line": f["line_start"], + "body": body, + } + ) + else: + general_comments.append(body) + + comment_path.write_text( + json.dumps( + { + "inline": inline_comments, + "general": general_comments, + }, + indent=2, + ) + ) + + manifest.append({"pr": slug, "status": "ok", "findings": len(findings)}) + except Exception as e: + print(f"[{i}/{len(prs)}] {slug} — ERROR: {e}", file=sys.stderr) + manifest.append({"pr": slug, "status": "failed", "reason": str(e)}) + + # Write run manifest + manifest_path = config.output_dir / "run_manifest.json" + manifest_path.write_text( + json.dumps( + { + "provenance": config.stamp(), + "timestamp": datetime.now(UTC).isoformat(), + "results": manifest, + }, + indent=2, + ) + ) + ok = sum(1 for m in manifest if m["status"] == "ok") + fail = sum(1 for m in manifest if m["status"] == "failed") + print(f"Done. {ok} reviewed, {fail} failed, {len(prs) - ok - fail} skipped.") + + +if __name__ == "__main__": + resume = "--resume" in sys.argv + run_all(resume=resume) diff --git a/benchmarks/martian/run_indexed.py b/benchmarks/martian/run_indexed.py new file mode 100644 index 0000000..62527a3 --- /dev/null +++ b/benchmarks/martian/run_indexed.py @@ -0,0 +1,324 @@ +# SPDX-License-Identifier: MIT +"""Step 2b: Run Grippy indexed reviews (with codebase tools + graph context). + +Same as run_grippy.py but builds CodebaseIndex + SQLiteGraphStore for each +repo clone, then passes CodebaseToolkit + graph context to the reviewer agent. +This mirrors the production review pipeline (review.py) as closely as possible. + +Requires repo clones at BENCH_REPOS_DIR (default: /tmp/grippy-bench-repos/). +""" + +from __future__ import annotations + +import itertools +import json +import re +import sys +from datetime import UTC, datetime +from pathlib import Path + +from benchmarks.martian.config import BenchConfig +from benchmarks.martian.fetch_diffs import parse_golden_prs +from benchmarks.martian.run_grippy import ( + _format_rules_for_agent, + format_finding_as_comment, + is_inline_finding, +) + +BENCH_REPOS_DIR = Path("/tmp/grippy-bench-repos") + + +def _extract_touched_files(diff_text: str) -> list[str]: + """Extract file paths from diff headers.""" + return re.findall(r"^diff --git a/.+ b/(.+)$", diff_text, re.MULTILINE) + + +def _repo_name_from_slug(slug: str) -> str: + """Extract repo name from slug like 'keycloak_PR32918'.""" + return slug.rsplit("_PR", 1)[0] + + +def _build_lite_toolkit(repo_root: Path) -> list: + """Create a filesystem-only toolkit (grep, read, list) without embeddings. + + Used when the embedding model is unavailable (e.g. VRAM contention). + This still provides the agent with codebase access for context-aware review, + just without semantic search_code. + """ + from agno.tools.function import Function + from agno.tools.toolkit import Toolkit + + from grippy.codebase import _make_grep_code, _make_list_files, _make_read_file + + tk = Toolkit(name="codebase") + for fn in [_make_grep_code(repo_root), _make_read_file(repo_root), _make_list_files(repo_root)]: + func = Function.from_callable(fn) + tk.functions[func.name] = func + return [tk] + + +def _probe_embedding_model(base_url: str, api_key: str) -> bool: + """Quick probe to check if the embedding model is available.""" + import httpx + + try: + resp = httpx.post( + f"{base_url}/embeddings", + json={"model": "text-embedding-qwen3-embedding-4b", "input": "test"}, + headers={"Authorization": f"Bearer {api_key}"}, + timeout=5.0, + ) + return resp.status_code == 200 + except Exception: + return False + + +def _build_codebase_tools( + repo_root: Path, + data_dir: Path, + config: BenchConfig, +) -> list: + """Build CodebaseToolkit. Falls back to lite (filesystem-only) on failure. + + Probes the embedding model first to avoid slow retry loops when the model + is unavailable (e.g. VRAM contention with the review model). + """ + if not _probe_embedding_model(config.base_url, config.api_key): + print(" Embedding model unavailable — using lite toolkit (grep/read/list)") + return _build_lite_toolkit(repo_root) + + try: + from agno.vectordb.lancedb import LanceDb + from agno.vectordb.search import SearchType + + from grippy.codebase import CodebaseIndex, CodebaseToolkit + from grippy.embedder import create_embedder + + cb_embedder = create_embedder( + transport=config.transport, + model="text-embedding-qwen3-embedding-4b", + base_url=config.base_url, + api_key=config.api_key, + ) + lance_dir = data_dir / "lance" + lance_dir.mkdir(parents=True, exist_ok=True) + + vector_db = LanceDb( + uri=str(lance_dir), + table_name="codebase_chunks", + search_type=SearchType.hybrid, + use_tantivy=False, + embedder=cb_embedder, + ) + cb_index = CodebaseIndex( + repo_root=repo_root, + vector_db=vector_db, + embedder=cb_embedder, + data_dir=data_dir, + ) + chunk_count = cb_index.build(force=False) + if chunk_count > 0: + print(f" Indexed {chunk_count} chunks") + else: + print(" Codebase index up-to-date (cached)") + + return [CodebaseToolkit(index=cb_index, repo_root=repo_root)] + except Exception as exc: + print(f" WARNING: Full index failed ({exc}), falling back to lite toolkit") + return _build_lite_toolkit(repo_root) + + +def _build_graph(repo_root: Path, data_dir: Path) -> "SQLiteGraphStore | None": + """Build import graph from .py files. Returns None if no Python files found.""" + from grippy.graph_store import SQLiteGraphStore + from grippy.imports import extract_imports + + graph = SQLiteGraphStore(db_path=data_dir / "navi-graph.db") + py_files = list(itertools.islice(repo_root.rglob("*.py"), 5000)) + if not py_files: + print(" No Python files — graph skipped") + return graph + + for py_f in py_files: + try: + rel = str(py_f.relative_to(repo_root)) + except ValueError: + continue + try: + imports = extract_imports(py_f) + except Exception: + continue + for imp in imports: + graph.add_edge( + src_type="FILE", + src_id=rel, + rel="IMPORTS", + dst_type="FILE", + dst_id=imp, + data={}, + ) + print(f" Graph: {len(py_files)} Python files scanned") + return graph + + +def review_single_pr_indexed( + *, + diff_text: str, + pr_title: str, + repo_root: Path, + data_dir: Path, + config: BenchConfig, +) -> dict: + """Run indexed Grippy review on a single diff.""" + from grippy.agent import create_reviewer, format_pr_context + from grippy.codebase import sanitize_tool_hook + from grippy.graph_context import build_context_pack, format_context_for_llm + from grippy.retry import run_review + from grippy.rules import load_profile, run_rules + + # Deterministic rules + profile_cfg = load_profile(cli_profile=config.profile) + rule_findings = run_rules(diff_text, profile_cfg) + rule_text = "" + if rule_findings and config.profile != "general": + rule_text = _format_rules_for_agent(rule_findings) + + # Build codebase tools (full index or lite filesystem fallback) + tools = _build_codebase_tools(repo_root, data_dir, config) + + # Build graph + context + graph_context_text = "" + try: + graph = _build_graph(repo_root, data_dir) + if graph: + touched = _extract_touched_files(diff_text) + pack = build_context_pack(graph, touched_files=touched, author_login="benchmark") + graph_context_text = format_context_for_llm(pack) + if graph_context_text: + print(f" Graph context: {len(graph_context_text)} chars") + except Exception as exc: + print(f" WARNING: Graph build failed: {exc}") + + agent = create_reviewer( + model_id=config.model_id, + base_url=config.base_url, + api_key=config.api_key, + transport=config.transport, + mode=config.mode, + tools=tools or None, + tool_call_limit=10 if tools else None, + tool_hooks=[sanitize_tool_hook] if tools else None, + include_rule_findings=bool(rule_text), + ) + message = format_pr_context( + title=pr_title, + author="benchmark", + branch="feature → main", + diff=diff_text, + rule_findings=rule_text, + file_context=graph_context_text, + ) + review = run_review(agent, message) + return review.model_dump() + + +def run_all_indexed(config: BenchConfig | None = None, resume: bool = False) -> None: + """Run indexed Grippy reviews on all fetched diffs.""" + config = config or BenchConfig.from_env() + prs = parse_golden_prs(config.golden_dir) + + diff_dir = config.output_dir / "diffs" + review_dir = config.output_dir / "reviews" + comment_dir = config.output_dir / "comments" + review_dir.mkdir(parents=True, exist_ok=True) + comment_dir.mkdir(parents=True, exist_ok=True) + + manifest = [] + for i, pr in enumerate(prs, 1): + slug = f"{pr['repo']}_PR{pr['pr_number']}" + review_path = review_dir / f"{slug}.json" + comment_path = comment_dir / f"{slug}.json" + + if resume and review_path.exists() and comment_path.exists(): + print(f"[{i}/{len(prs)}] {slug} — skipped (resume)") + continue + + diff_path = diff_dir / f"{slug}.diff" + if not diff_path.exists(): + # Try shared diff dir (output/ parent) + shared_diff_dir = config.output_dir.parent / "output" / "diffs" + diff_path = shared_diff_dir / f"{slug}.diff" + + if not diff_path.exists(): + print(f"[{i}/{len(prs)}] {slug} — ERROR: diff not found", file=sys.stderr) + manifest.append({"pr": slug, "status": "failed", "reason": "diff_not_found"}) + continue + + repo_name = _repo_name_from_slug(slug) + repo_root = BENCH_REPOS_DIR / repo_name + if not repo_root.exists(): + print(f"[{i}/{len(prs)}] {slug} — ERROR: repo clone not found at {repo_root}") + manifest.append({"pr": slug, "status": "failed", "reason": "repo_not_found"}) + continue + + diff_text = diff_path.read_text() + data_dir = config.output_dir / "data" / repo_name + data_dir.mkdir(parents=True, exist_ok=True) + print(f"[{i}/{len(prs)}] {slug} — indexed review ({len(diff_text)} chars)") + + ts = datetime.now(UTC).isoformat() + try: + review_data = review_single_pr_indexed( + diff_text=diff_text, + pr_title=pr["pr_title"], + repo_root=repo_root, + data_dir=data_dir, + config=config, + ) + + output = {"provenance": config.stamp(), "timestamp": ts, "review": review_data} + review_path.write_text(json.dumps(output, indent=2)) + + findings = review_data.get("findings", []) + inline_comments = [] + general_comments = [] + for f in findings: + body = format_finding_as_comment(f) + if is_inline_finding(f): + inline_comments.append( + {"path": f["file"], "line": f["line_start"], "body": body} + ) + else: + general_comments.append(body) + + comment_path.write_text( + json.dumps({"inline": inline_comments, "general": general_comments}, indent=2) + ) + manifest.append({"pr": slug, "status": "ok", "findings": len(findings)}) + except Exception as e: + print(f"[{i}/{len(prs)}] {slug} — ERROR: {e}", file=sys.stderr) + manifest.append({"pr": slug, "status": "failed", "reason": str(e)}) + + manifest_path = config.output_dir / "run_manifest.json" + manifest_path.write_text( + json.dumps( + {"provenance": config.stamp(), "timestamp": datetime.now(UTC).isoformat(), "results": manifest}, + indent=2, + ) + ) + ok = sum(1 for m in manifest if m["status"] == "ok") + fail = sum(1 for m in manifest if m["status"] == "failed") + print(f"Done. {ok} reviewed, {fail} failed, {len(prs) - ok - fail} skipped.") + + +if __name__ == "__main__": + import os + + output_dir = Path(os.environ.get("BENCH_OUTPUT_DIR", str(Path(__file__).parent / "output-devstral-indexed"))) + model_id = os.environ.get("GRIPPY_MODEL_ID", "devstral-small-2-24b-instruct-2512") + config = BenchConfig( + model_id=model_id, + output_dir=output_dir, + ) + resume = "--resume" in sys.argv + run_all_indexed(config=config, resume=resume) diff --git a/pyproject.toml b/pyproject.toml index 7368619..d091c34 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -55,6 +55,8 @@ groq = ["agno[groq]>=1.1.0"] mistral = ["agno[mistral]>=1.1.0"] benchmarks = [ "coir-eval>=0.1.0", + "anthropic>=0.40", + "httpx>=0.27", ] [project.scripts] diff --git a/src/grippy/agent.py b/src/grippy/agent.py index 368b33c..691634c 100644 --- a/src/grippy/agent.py +++ b/src/grippy/agent.py @@ -19,6 +19,44 @@ DEFAULT_PROMPTS_DIR = Path(__file__).parent / "prompts_data" + +# --------------------------------------------------------------------------- +# Local-endpoint model subclass — fixes tool + structured-output conflict +# --------------------------------------------------------------------------- + + +class _LocalModel(OpenAILike): + """OpenAILike variant that suppresses response_format when tools are active. + + Local inference servers (LM Studio, Ollama, vLLM) cannot combine structured + output grammars (response_format) with tool-calling grammars in the same + request. This subclass strips response_format from the API params when + tools are present, relying instead on: + + 1. Agno's system-prompt JSON output instructions (auto-injected when + supports_native_structured_outputs is False), and + 2. The retry layer in retry.py for JSON parsing + Pydantic validation. + + When no tools are present, response_format passes through normally. + """ + + def get_request_params( + self, + response_format: Any = None, + tools: Any = None, + tool_choice: Any = None, + run_response: Any = None, + ) -> dict[str, Any]: + params = super().get_request_params( + response_format=response_format, + tools=tools, + tool_choice=tool_choice, + run_response=run_response, + ) + if "tools" in params: + params.pop("response_format", None) + return params + # --------------------------------------------------------------------------- # Provider registry — maps transport names to agno model classes. # Each entry: (module_path, class_name, supports_native_structured_outputs) @@ -191,7 +229,13 @@ def create_reviewer( log.info("Grippy transport=%s (source: %s)", resolved_transport, source) if resolved_transport == "local": - model = OpenAILike(id=model_id, api_key=api_key, base_url=base_url) + model = _LocalModel(id=model_id, api_key=api_key, base_url=base_url) + # Local endpoints do not support native structured outputs. Setting + # this to False makes Agno inject JSON schema instructions into the + # system prompt instead. _LocalModel.get_request_params() separately + # strips response_format when tools are active to avoid the grammar + # conflict (LM Studio cannot combine both in one request). + model.supports_native_structured_outputs = False structured = False else: # Deferred import from provider registry @@ -200,6 +244,15 @@ def create_reviewer( model_cls = getattr(mod, class_name) model = model_cls(id=model_id) + # Only pass output_schema when the provider supports it natively or has a + # response_format stripping mechanism (_LocalModel). For other providers + # (Anthropic, Google, Groq, Mistral) the prompt chain (output-schema.md) + # already contains the full JSON schema, and retry.py handles parsing + + # Pydantic validation independently. Passing output_schema to Agno for + # these providers causes a "compiled grammar is too large" API error + # because Agno sends the schema as response_format. + output_schema = GrippyReview if (structured or resolved_transport == "local") else None + # Security: session history is NEVER re-injected into the LLM context, # regardless of whether a session db is configured. Prior run responses # may contain attacker-controlled PR content echoed by the model — @@ -214,7 +267,7 @@ def create_reviewer( mode=mode, include_rule_findings=include_rule_findings, ), - output_schema=GrippyReview, + output_schema=output_schema, structured_outputs=structured, add_history_to_context=False, markdown=False, diff --git a/src/grippy/retry.py b/src/grippy/retry.py index 5171a23..7eaeed2 100644 --- a/src/grippy/retry.py +++ b/src/grippy/retry.py @@ -151,9 +151,11 @@ def run_review( errors.append(f"Attempt {attempt}: {error_str}") if attempt <= max_retries: current_message = ( - f"Your output is missing findings for these rule-detected issues: " - f"{', '.join(missing)}. " - f"Every rule finding MUST appear with its rule_id set." + f"CORRECTION: Your output is missing findings for these " + f"rule-detected issues: {', '.join(missing)}. " + f"Every rule finding MUST appear with its rule_id set.\n\n" + f"Review the PR again and produce a complete response:\n\n" + f"{message}" ) continue # Final attempt still missing — warn but return what we have @@ -187,10 +189,11 @@ def run_review( safe_summary = "Value error in response" current_message = ( - f"Your previous output failed validation. " + f"CORRECTION: Your previous output failed validation. " f"Error: {safe_summary}\n\n" - f"Please fix the errors and output a valid JSON object " - f"matching the GrippyReview schema. Output ONLY the JSON." + f"Fix the errors and produce a valid JSON object matching " + f"the GrippyReview schema. Review the PR again:\n\n" + f"{message}" ) raise ReviewParseError( diff --git a/tests/test_bench_martian_config.py b/tests/test_bench_martian_config.py new file mode 100644 index 0000000..52f0a9a --- /dev/null +++ b/tests/test_bench_martian_config.py @@ -0,0 +1,55 @@ +# SPDX-License-Identifier: MIT +"""Tests for benchmarks/martian/config.py.""" + +from benchmarks.martian.config import MARTIAN_COMMIT_SHA, BenchConfig + + +def test_martian_commit_sha_is_pinned(): + assert len(MARTIAN_COMMIT_SHA) == 40 + assert MARTIAN_COMMIT_SHA == "012d68202e56280855b85abb956410086008a7b2" + + +def test_default_config(): + cfg = BenchConfig() + assert cfg.profile == "security" + assert cfg.golden_dir.name == "golden_comments" + assert cfg.output_dir.name == "output" + + +def test_config_from_env(monkeypatch): + monkeypatch.setenv("GRIPPY_MODEL_ID", "test-model") + monkeypatch.setenv("GRIPPY_TRANSPORT", "openai") + cfg = BenchConfig.from_env() + assert cfg.model_id == "test-model" + assert cfg.transport == "openai" + + +def test_config_is_frozen(): + cfg = BenchConfig() + try: + cfg.model_id = "changed" # type: ignore[misc] + raise AssertionError("Should raise FrozenInstanceError") + except AttributeError: + pass + + +def test_stamp_has_required_fields(): + cfg = BenchConfig() + stamp = cfg.stamp() + assert "grippy_version" in stamp + assert "harness_git_sha" in stamp + assert "model_id" in stamp + assert "martian_commit" in stamp + assert "extract_model" in stamp + assert "judge_model" in stamp + assert "prompt_checksums" in stamp + assert stamp["extract_model"] == cfg.extract_model + assert stamp["judge_model"] == cfg.judge_model + + +def test_stamp_prompt_checksums_are_full_sha256(): + """Prompt checksums must be full 64-char SHA-256, not truncated.""" + cfg = BenchConfig() + stamp = cfg.stamp() + for name, checksum in stamp["prompt_checksums"].items(): + assert len(checksum) == 64, f"Checksum for {name} is truncated: {len(checksum)} chars" diff --git a/tests/test_bench_martian_contracts.py b/tests/test_bench_martian_contracts.py new file mode 100644 index 0000000..37ef118 --- /dev/null +++ b/tests/test_bench_martian_contracts.py @@ -0,0 +1,100 @@ +# SPDX-License-Identifier: MIT +"""Contract tests: verify harness dependencies are importable and shaped correctly.""" + + +def test_grippy_imports(): + """Verify the harness can import Grippy's public API surface.""" + from grippy.agent import create_reviewer, format_pr_context + from grippy.retry import run_review + from grippy.rules import load_profile, run_rules + from grippy.schema import GrippyReview + + # Verify callables + assert callable(create_reviewer) + assert callable(format_pr_context) + assert callable(run_review) + assert callable(run_rules) + assert callable(load_profile) + + # Verify GrippyReview has expected fields + assert hasattr(GrippyReview, "model_fields") + assert "findings" in GrippyReview.model_fields + + +def test_anthropic_sdk_response_shape(): + """Verify Anthropic SDK has the response shape the harness depends on. + + The harness accesses resp.content[0].text — this test proves + the SDK types actually expose that path without making an API call. + """ + import anthropic + from anthropic.types import Message, TextBlock, Usage + + assert hasattr(anthropic, "Anthropic") + + # Verify TextBlock has .text attribute (harness reads resp.content[0].text) + block = TextBlock(type="text", text='{"reasoning": "test", "match": true, "confidence": 0.9}') + assert hasattr(block, "text") + assert isinstance(block.text, str) + + # Verify the text can be parsed as the judge response format + import json + + parsed = json.loads(block.text) + assert parsed["match"] is True + assert isinstance(parsed["confidence"], float) + assert "reasoning" in parsed + + # Verify Message.content is a list of ContentBlock (harness indexes [0]) + msg = Message( + id="msg_test", + type="message", + role="assistant", + content=[block], + model="test", + stop_reason="end_turn", + usage=Usage(input_tokens=0, output_tokens=0), + ) + assert isinstance(msg.content, list) + assert len(msg.content) == 1 + assert hasattr(msg.content[0], "text") + + +def test_vendored_prompts_loadable(): + """Verify vendored prompt files exist and are non-empty.""" + from pathlib import Path + + prompts_dir = Path(__file__).resolve().parent.parent / "benchmarks" / "martian" / "prompts" + for name in ("extract.txt", "extract_system.txt", "judge.txt"): + path = prompts_dir / name + assert path.exists(), f"Missing vendored prompt: {name}" + content = path.read_text().strip() + assert len(content) > 50, f"Prompt {name} is suspiciously short" + + +def test_vendored_prompt_checksums_match(): + """Verify nobody edited vendored prompts without updating checksums. + + If this fails, either re-vendor from Martian or update CHECKSUMS.sha256. + Never edit prompts in place. + """ + import hashlib + from pathlib import Path + + prompts_dir = Path(__file__).resolve().parent.parent / "benchmarks" / "martian" / "prompts" + checksum_file = prompts_dir / "CHECKSUMS.sha256" + assert checksum_file.exists(), "Missing CHECKSUMS.sha256" + + expected = {} + for line in checksum_file.read_text().strip().splitlines(): + parts = line.split() + if len(parts) == 2: + expected[parts[1]] = parts[0] + + for name, expected_hash in expected.items(): + path = prompts_dir / name + actual_hash = hashlib.sha256(path.read_bytes()).hexdigest() + assert actual_hash == expected_hash, ( + f"Checksum mismatch for {name}: expected {expected_hash[:16]}..., " + f"got {actual_hash[:16]}... — re-vendor from Martian, don't edit in place" + ) diff --git a/tests/test_bench_martian_extract.py b/tests/test_bench_martian_extract.py new file mode 100644 index 0000000..1493712 --- /dev/null +++ b/tests/test_bench_martian_extract.py @@ -0,0 +1,57 @@ +# SPDX-License-Identifier: MIT +"""Tests for benchmarks/martian/extract.py.""" + +from benchmarks.martian.extract import ( + EXTRACT_SYSTEM_PROMPT, + EXTRACT_USER_PROMPT, + extract_candidates_for_pr, + inline_to_candidate, +) + + +def test_extract_prompt_matches_martian(): + """Verify prompts are vendored verbatim from Martian.""" + assert "Extract each distinct code issue" in EXTRACT_USER_PROMPT + assert "Each issue should be a single, specific problem" in EXTRACT_USER_PROMPT + assert "You extract code review issues" in EXTRACT_SYSTEM_PROMPT + + +def test_inline_to_candidate(): + inline = {"path": "src/auth.py", "line": 42, "body": "SQL injection risk"} + candidate = inline_to_candidate(inline) + assert candidate["text"] == "SQL injection risk" + assert candidate["path"] == "src/auth.py" + assert candidate["line"] == 42 + assert candidate["source"] == "inline" + + +def test_extract_candidates_all_inline(): + """When all findings are inline, no LLM extraction needed.""" + comments = { + "inline": [ + {"path": "a.py", "line": 1, "body": "Issue one"}, + {"path": "b.py", "line": 2, "body": "Issue two"}, + ], + "general": [], + } + candidates = extract_candidates_for_pr(comments, llm_extract_fn=None) + assert len(candidates) == 2 + assert all(c["source"] == "inline" for c in candidates) + + +def test_extract_candidates_with_general(): + """General comments go through LLM extraction.""" + comments = { + "inline": [{"path": "a.py", "line": 1, "body": "Inline issue"}], + "general": ["Found two problems: null check missing and race condition"], + } + + def mock_extract(text): + return ["null check missing", "race condition"] + + candidates = extract_candidates_for_pr(comments, llm_extract_fn=mock_extract) + assert len(candidates) == 3 + assert candidates[0]["source"] == "inline" + assert candidates[1]["source"] == "extracted" + assert candidates[2]["source"] == "extracted" + assert candidates[1]["text"] == "null check missing" diff --git a/tests/test_bench_martian_fetch.py b/tests/test_bench_martian_fetch.py new file mode 100644 index 0000000..f2ead1a --- /dev/null +++ b/tests/test_bench_martian_fetch.py @@ -0,0 +1,62 @@ +# SPDX-License-Identifier: MIT +"""Tests for benchmarks/martian/fetch_diffs.py.""" + +import json + +from benchmarks.martian.fetch_diffs import fetch_diff, parse_golden_prs + +SAMPLE_GOLDEN = [ + { + "pr_title": "Fix auth bug", + "url": "https://github.com/keycloak/keycloak/pull/32918", + "comments": [{"comment": "issue", "severity": "Critical"}], + } +] + + +def test_parse_golden_prs(tmp_path): + golden_dir = tmp_path / "golden_comments" + golden_dir.mkdir() + (golden_dir / "keycloak.json").write_text(json.dumps(SAMPLE_GOLDEN)) + + prs = parse_golden_prs(golden_dir) + assert len(prs) == 1 + assert prs[0]["owner"] == "keycloak" + assert prs[0]["repo"] == "keycloak" + assert prs[0]["pr_number"] == 32918 + assert prs[0]["pr_title"] == "Fix auth bug" + assert prs[0]["golden_url"] == "https://github.com/keycloak/keycloak/pull/32918" + + +def test_parse_golden_extracts_owner_repo_from_url(tmp_path): + """Handles orgs like calcom/cal.com correctly.""" + golden_dir = tmp_path / "golden_comments" + golden_dir.mkdir() + data = [ + { + "pr_title": "Test", + "url": "https://github.com/calcom/cal.com/pull/123", + "comments": [], + } + ] + (golden_dir / "cal_dot_com.json").write_text(json.dumps(data)) + prs = parse_golden_prs(golden_dir) + assert prs[0]["owner"] == "calcom" + assert prs[0]["repo"] == "cal.com" + + +def test_fetch_diff_caching(tmp_path): + """Skips fetch if diff file already exists.""" + diff_dir = tmp_path / "diffs" + diff_dir.mkdir() + cached = diff_dir / "keycloak_PR32918.diff" + cached.write_text("cached diff content") + + result = fetch_diff( + owner="keycloak", + repo="keycloak", + pr_number=32918, + output_dir=diff_dir, + token="fake", + ) + assert result == "cached diff content" diff --git a/tests/test_bench_martian_judge.py b/tests/test_bench_martian_judge.py new file mode 100644 index 0000000..6a28e86 --- /dev/null +++ b/tests/test_bench_martian_judge.py @@ -0,0 +1,62 @@ +# SPDX-License-Identifier: MIT +"""Tests for benchmarks/martian/judge.py.""" + +import pytest + +from benchmarks.martian.judge import ( + JUDGE_PROMPT, + compute_metrics, + judge_pair, + match_candidates_to_golden, +) + + +def test_judge_prompt_matches_martian(): + assert "Determine if the candidate identifies the SAME underlying issue" in JUDGE_PROMPT + assert "Accept semantic matches" in JUDGE_PROMPT + + +def test_match_candidates_greedy_assignment(): + """Each golden matches at most one candidate (highest confidence).""" + verdicts = [ + {"golden_idx": 0, "candidate_idx": 0, "match": True, "confidence": 0.9}, + {"golden_idx": 0, "candidate_idx": 1, "match": True, "confidence": 0.7}, + {"golden_idx": 1, "candidate_idx": 0, "match": True, "confidence": 0.8}, + {"golden_idx": 1, "candidate_idx": 1, "match": False, "confidence": 0.3}, + ] + matches = match_candidates_to_golden(verdicts, n_golden=2, n_candidates=2) + # Golden 0 matches candidate 0 (conf 0.9) + # Golden 1 must match candidate 1 (candidate 0 already taken) — but conf 0.3, no match + assert matches[0] == 0 # golden 0 → candidate 0 + assert matches[1] is None # golden 1 → no match (candidate 0 taken, candidate 1 no match) + + +def test_compute_metrics_perfect(): + metrics = compute_metrics(tp=3, total_candidates=3, total_golden=3) + assert metrics["precision"] == 1.0 + assert metrics["recall"] == 1.0 + assert metrics["f1"] == 1.0 + + +def test_compute_metrics_partial(): + metrics = compute_metrics(tp=2, total_candidates=4, total_golden=3) + assert metrics["precision"] == pytest.approx(0.5) + assert metrics["recall"] == pytest.approx(2 / 3, abs=0.01) + + +def test_compute_metrics_zero_candidates(): + metrics = compute_metrics(tp=0, total_candidates=0, total_golden=3) + assert metrics["precision"] == 0.0 + assert metrics["recall"] == 0.0 + assert metrics["f1"] == 0.0 + + +def test_judge_pair_with_custom_fn(): + """judge_pair uses custom fn when provided.""" + + def mock_judge(golden, candidate): + return {"reasoning": "mock", "match": True, "confidence": 0.95} + + result = judge_pair("golden text", "candidate text", judge_fn=mock_judge) + assert result["match"] is True + assert result["confidence"] == 0.95 diff --git a/tests/test_bench_martian_report.py b/tests/test_bench_martian_report.py new file mode 100644 index 0000000..0fe762e --- /dev/null +++ b/tests/test_bench_martian_report.py @@ -0,0 +1,207 @@ +# SPDX-License-Identifier: MIT +"""Tests for benchmarks/martian/report.py.""" + +import json + +from benchmarks.martian.report import aggregate_by_repo, format_table, report + +SAMPLE_RESULTS = { + "overall": {"precision": 0.6, "recall": 0.5, "f1": 0.545, "tp": 5}, + "per_pr": [ + { + "pr": "keycloak_PR32918", + "golden_url": "https://github.com/keycloak/keycloak/pull/32918", + "n_candidates": 3, + "n_golden": 2, + "metrics": {"precision": 0.67, "recall": 1.0, "f1": 0.8, "tp": 2}, + }, + { + "pr": "sentry_PR100", + "golden_url": "https://github.com/getsentry/sentry/pull/100", + "n_candidates": 5, + "n_golden": 4, + "metrics": {"precision": 0.6, "recall": 0.75, "f1": 0.67, "tp": 3}, + }, + ], +} + + +def test_aggregate_by_repo(): + by_repo = aggregate_by_repo(SAMPLE_RESULTS["per_pr"]) + assert "keycloak" in by_repo + assert "sentry" in by_repo + assert by_repo["keycloak"]["n_prs"] == 1 + + +def test_format_table_produces_lines(): + lines = format_table(SAMPLE_RESULTS) + text = "\n".join(lines) + assert "Precision" in text + assert "Recall" in text + assert "F1" in text + + +def test_report_unified_failure_accounting(tmp_path, capsys): + """Failures from run + extract + judge phases all surface in one report.""" + output_dir = tmp_path / "output" + scores_dir = output_dir / "scores" + scores_dir.mkdir(parents=True) + + # Judge results: one normal, one missing_candidates + judge_results = { + "overall": {"precision": 0.5, "recall": 0.5, "f1": 0.5, "tp": 1}, + "per_pr": [ + { + "pr": "keycloak_PR100", + "golden_url": "https://github.com/keycloak/keycloak/pull/100", + "n_candidates": 2, + "n_golden": 2, + "metrics": {"precision": 0.5, "recall": 0.5, "f1": 0.5, "tp": 1}, + }, + { + "pr": "sentry_PR200", + "golden_url": "https://github.com/getsentry/sentry/pull/200", + "n_candidates": 0, + "n_golden": 3, + "status": "missing_candidates", + "metrics": {"precision": 0.0, "recall": 0.0, "f1": 0.0, "tp": 0}, + }, + ], + } + (scores_dir / "judge_results.json").write_text(json.dumps(judge_results)) + + # Run manifest: one failure + run_manifest = { + "results": [ + {"pr": "grafana_PR300", "status": "failed", "reason": "timeout"}, + {"pr": "keycloak_PR100", "status": "ok", "findings": 3}, + ] + } + (output_dir / "run_manifest.json").write_text(json.dumps(run_manifest)) + + # Extract manifest: another failure + extract_manifest = [ + {"pr": "discourse_PR400", "status": "failed", "reason": "json_parse_error"}, + ] + (output_dir / "extract_manifest.json").write_text(json.dumps(extract_manifest)) + + from benchmarks.martian.config import BenchConfig + + cfg = BenchConfig(output_dir=output_dir) + report(config=cfg) + + captured = capsys.readouterr().out + # All three failure phases should appear + assert "[run] grafana_PR300" in captured + assert "[extract] discourse_PR400" in captured + assert "[judge] sentry_PR200" in captured + assert "FAILURE ACCOUNTING" in captured + + +def test_report_unique_failure_dedup(tmp_path, capsys): + """Same PR failing in run AND extract counts as ONE unique failure.""" + output_dir = tmp_path / "output" + scores_dir = output_dir / "scores" + scores_dir.mkdir(parents=True) + + judge_results = { + "overall": {"precision": 0.0, "recall": 0.0, "f1": 0.0, "tp": 0}, + "per_pr": [ + { + "pr": "keycloak_PR100", + "golden_url": "https://github.com/keycloak/keycloak/pull/100", + "n_candidates": 0, + "n_golden": 2, + "status": "missing_candidates", + "metrics": {"precision": 0.0, "recall": 0.0, "f1": 0.0, "tp": 0}, + }, + ], + } + (scores_dir / "judge_results.json").write_text(json.dumps(judge_results)) + + # Same PR fails in both run and extract + run_manifest = {"results": [{"pr": "keycloak_PR100", "status": "failed", "reason": "timeout"}]} + (output_dir / "run_manifest.json").write_text(json.dumps(run_manifest)) + + extract_manifest = [{"pr": "keycloak_PR100", "status": "failed", "reason": "no_comments"}] + (output_dir / "extract_manifest.json").write_text(json.dumps(extract_manifest)) + + from benchmarks.martian.config import BenchConfig + + cfg = BenchConfig(output_dir=output_dir) + report(config=cfg) + + captured = capsys.readouterr().out + # Should count as 1 failure, not 3 + assert "1/1 failed" in captured + # First occurrence wins (run phase) + assert "[run] keycloak_PR100" in captured + + +def test_report_high_failure_rate_warning(tmp_path, capsys): + """>10% failure rate triggers the public-claims warning.""" + output_dir = tmp_path / "output" + scores_dir = output_dir / "scores" + scores_dir.mkdir(parents=True) + + # 2 PRs, 1 missing = 50% failure rate + judge_results = { + "overall": {"precision": 0.5, "recall": 0.5, "f1": 0.5, "tp": 1}, + "per_pr": [ + { + "pr": "keycloak_PR100", + "golden_url": "https://github.com/keycloak/keycloak/pull/100", + "n_candidates": 2, + "n_golden": 2, + "metrics": {"precision": 0.5, "recall": 0.5, "f1": 0.5, "tp": 1}, + }, + { + "pr": "sentry_PR200", + "golden_url": "https://github.com/getsentry/sentry/pull/200", + "n_candidates": 0, + "n_golden": 3, + "status": "missing_candidates", + "metrics": {"precision": 0.0, "recall": 0.0, "f1": 0.0, "tp": 0}, + }, + ], + } + (scores_dir / "judge_results.json").write_text(json.dumps(judge_results)) + + from benchmarks.martian.config import BenchConfig + + cfg = BenchConfig(output_dir=output_dir) + report(config=cfg) + + captured = capsys.readouterr().out + assert "WARNING" in captured + assert "NOT valid for public claims" in captured + + +def test_report_no_failures_clean_output(tmp_path, capsys): + """Clean run shows 'No failures' message.""" + output_dir = tmp_path / "output" + scores_dir = output_dir / "scores" + scores_dir.mkdir(parents=True) + + judge_results = { + "overall": {"precision": 0.8, "recall": 0.7, "f1": 0.74, "tp": 7}, + "per_pr": [ + { + "pr": "keycloak_PR100", + "golden_url": "https://github.com/keycloak/keycloak/pull/100", + "n_candidates": 3, + "n_golden": 2, + "metrics": {"precision": 0.67, "recall": 1.0, "f1": 0.8, "tp": 2}, + }, + ], + } + (scores_dir / "judge_results.json").write_text(json.dumps(judge_results)) + + from benchmarks.martian.config import BenchConfig + + cfg = BenchConfig(output_dir=output_dir) + report(config=cfg) + + captured = capsys.readouterr().out + assert "No failures" in captured + assert "FAILURE ACCOUNTING" not in captured diff --git a/tests/test_bench_martian_run.py b/tests/test_bench_martian_run.py new file mode 100644 index 0000000..c151c85 --- /dev/null +++ b/tests/test_bench_martian_run.py @@ -0,0 +1,91 @@ +# SPDX-License-Identifier: MIT +"""Tests for benchmarks/martian/run_grippy.py.""" + +from benchmarks.martian.run_grippy import ( + format_finding_as_comment, + is_inline_finding, +) + + +def _make_finding(**overrides): + """Create a minimal Finding-like dict.""" + base = { + "id": "F-001", + "severity": "HIGH", + "confidence": 85, + "category": "security", + "file": "src/auth.py", + "line_start": 42, + "line_end": 42, + "title": "SQL injection risk", + "description": "Query concatenates user input without parameterization.", + "suggestion": "Use parameterized queries.", + "evidence": "line 42: query = f'SELECT * FROM users WHERE id={user_id}'", + "grippy_note": "Watch out!", + } + base.update(overrides) + return base + + +def test_is_inline_finding(): + assert is_inline_finding(_make_finding()) is True + assert is_inline_finding(_make_finding(file="", line_start=0)) is False + assert is_inline_finding(_make_finding(file="src/a.py", line_start=0)) is False + + +def test_format_finding_as_comment_inline(): + finding = _make_finding() + comment = format_finding_as_comment(finding) + assert "SQL injection risk" in comment + assert "concatenates user input" in comment + # Must NOT contain severity tags, suggestions, evidence, confidence + assert "[HIGH]" not in comment + assert "Suggestion:" not in comment + assert "Evidence:" not in comment + assert "85" not in comment + + +def test_format_finding_as_comment_is_concise(): + finding = _make_finding() + comment = format_finding_as_comment(finding) + # Should be markdown heading + description, nothing else + lines = [line for line in comment.strip().split("\n") if line.strip()] + assert len(lines) <= 4 # heading + description (possibly wrapped) + + +def test_format_finding_parity_with_production(): + """Verify benchmark formatter output is structurally consistent + with production build_review_comment() — same substance, less metadata.""" + from grippy.github_review import build_review_comment + from grippy.schema import Finding, FindingCategory, Severity + + prod_finding = Finding( + id="F-001", + severity=Severity.HIGH, + confidence=85, + category=FindingCategory.SECURITY, + file="src/auth.py", + line_start=42, + line_end=42, + title="SQL injection risk", + description="Query concatenates user input without parameterization.", + suggestion="Use parameterized queries.", + evidence="line 42", + grippy_note="Watch out!", + ) + + prod_comment = build_review_comment(prod_finding) + bench_finding = _make_finding() + bench_comment = format_finding_as_comment(bench_finding) + + # Both must contain the title and description + assert "SQL injection risk" in prod_comment["body"] + assert "SQL injection risk" in bench_comment + assert "concatenates user input" in prod_comment["body"] + assert "concatenates user input" in bench_comment + + # Bench version must NOT contain metadata that production includes + assert "Confidence:" in prod_comment["body"] # production has it + assert "Confidence" not in bench_comment # bench strips it + assert "Suggestion:" in prod_comment["body"] # production has it + assert "Suggestion" not in bench_comment # bench strips it diff --git a/tests/test_grippy_agent.py b/tests/test_grippy_agent.py index 81086c0..94d25d9 100644 --- a/tests/test_grippy_agent.py +++ b/tests/test_grippy_agent.py @@ -1,9 +1,9 @@ # SPDX-License-Identifier: MIT -"""Tests for Grippy agent utilities (format_pr_context).""" +"""Tests for Grippy agent utilities (format_pr_context, _LocalModel).""" from __future__ import annotations -from grippy.agent import _escape_xml, format_pr_context +from grippy.agent import _escape_xml, _LocalModel, create_reviewer, format_pr_context # --- Sample diff for testing --- @@ -271,3 +271,90 @@ def test_passthrough_clean_text(self) -> None: def test_empty_string(self) -> None: assert _escape_xml("") == "" + + +class TestOutputSchemaConditional: + """Verify output_schema is suppressed for non-structured, non-local providers.""" + + def test_local_transport_gets_output_schema(self) -> None: + """Local transport uses _LocalModel which handles response_format stripping.""" + from grippy.schema import GrippyReview + + agent = create_reviewer(transport="local") + assert agent.output_schema == GrippyReview + + def test_openai_transport_gets_output_schema(self) -> None: + """OpenAI supports native structured outputs — output_schema should be set.""" + import os + + from grippy.schema import GrippyReview + + os.environ["OPENAI_API_KEY"] = "test-key" # pragma: allowlist secret + try: + agent = create_reviewer(transport="openai", model_id="gpt-4o") + assert agent.output_schema == GrippyReview + finally: + del os.environ["OPENAI_API_KEY"] + + def test_anthropic_transport_skips_output_schema(self) -> None: + """Anthropic rejects large compiled grammars — output_schema must be None.""" + import os + + os.environ["ANTHROPIC_API_KEY"] = "test-key" # pragma: allowlist secret + try: + agent = create_reviewer(transport="anthropic", model_id="claude-sonnet-4-5-20250929") + assert agent.output_schema is None + finally: + del os.environ["ANTHROPIC_API_KEY"] + + +class TestLocalModel: + """Regression tests for _LocalModel tool + structured-output conflict fix.""" + + def test_strips_response_format_when_tools_present(self) -> None: + """LM Studio cannot combine response_format with tool grammars.""" + model = _LocalModel(id="test-model", api_key="test", base_url="http://localhost:1234/v1") + params = model.get_request_params( + response_format={"type": "json_object"}, + tools=[{"type": "function", "function": {"name": "read_file"}}], + ) + assert "response_format" not in params + assert "tools" in params + + def test_keeps_response_format_when_no_tools(self) -> None: + """Without tools, response_format should pass through normally.""" + model = _LocalModel(id="test-model", api_key="test", base_url="http://localhost:1234/v1") + params = model.get_request_params( + response_format={"type": "json_object"}, + tools=None, + ) + assert params["response_format"] == {"type": "json_object"} + + def test_strips_response_format_with_pydantic_schema(self) -> None: + """JSON schema response_format (Pydantic class) also stripped with tools.""" + from grippy.schema import GrippyReview + + model = _LocalModel(id="test-model", api_key="test", base_url="http://localhost:1234/v1") + params = model.get_request_params( + response_format=GrippyReview, + tools=[{"type": "function", "function": {"name": "grep_code"}}], + ) + assert "response_format" not in params + assert "tools" in params + + def test_no_tools_no_response_format_passthrough(self) -> None: + """Neither tools nor response_format — clean passthrough.""" + model = _LocalModel(id="test-model", api_key="test", base_url="http://localhost:1234/v1") + params = model.get_request_params() + assert "response_format" not in params + assert "tools" not in params + + def test_empty_tools_list_preserves_response_format(self) -> None: + """Empty tools list should not trigger stripping.""" + model = _LocalModel(id="test-model", api_key="test", base_url="http://localhost:1234/v1") + params = model.get_request_params( + response_format={"type": "json_object"}, + tools=[], + ) + # Empty list → OpenAIChat doesn't add "tools" to params + assert params.get("response_format") == {"type": "json_object"} diff --git a/uv.lock b/uv.lock index 845cde1..2e99abe 100644 --- a/uv.lock +++ b/uv.lock @@ -1016,7 +1016,9 @@ anthropic = [ { name = "agno", extra = ["anthropic"] }, ] benchmarks = [ + { name = "anthropic" }, { name = "coir-eval" }, + { name = "httpx" }, ] google = [ { name = "agno", extra = ["google"] }, @@ -1053,7 +1055,9 @@ requires-dist = [ { name = "agno", extras = ["groq"], marker = "extra == 'groq'", specifier = ">=1.1.0" }, { name = "agno", extras = ["mistral"], marker = "extra == 'mistral'", specifier = ">=1.1.0" }, { name = "agno", extras = ["openai"], specifier = ">=1.1.0" }, + { name = "anthropic", marker = "extra == 'benchmarks'", specifier = ">=0.40" }, { name = "coir-eval", marker = "extra == 'benchmarks'", specifier = ">=0.1.0" }, + { name = "httpx", marker = "extra == 'benchmarks'", specifier = ">=0.27" }, { name = "lancedb", specifier = ">=0.20.0" }, { name = "mcp", specifier = ">=1.0.0" }, { name = "navi-sanitize", specifier = ">=0.1.0" }, @@ -3836,6 +3840,11 @@ dependencies = [ wheels = [ { url = "https://files.pythonhosted.org/packages/d3/54/a2ba279afcca44bbd320d4e73675b282fcee3d81400ea1b53934efca6462/torch-2.10.0-2-cp312-none-macosx_11_0_arm64.whl", hash = "sha256:13ec4add8c3faaed8d13e0574f5cd4a323c11655546f91fbe6afa77b57423574", size = 79498202, upload-time = "2026-02-10T21:44:52.603Z" }, { url = "https://files.pythonhosted.org/packages/ec/23/2c9fe0c9c27f7f6cb865abcea8a4568f29f00acaeadfc6a37f6801f84cb4/torch-2.10.0-2-cp313-none-macosx_11_0_arm64.whl", hash = "sha256:e521c9f030a3774ed770a9c011751fb47c4d12029a3d6522116e48431f2ff89e", size = 79498254, upload-time = "2026-02-10T21:44:44.095Z" }, + { url = "https://files.pythonhosted.org/packages/b3/7a/abada41517ce0011775f0f4eacc79659bc9bc6c361e6bfe6f7052a6b9363/torch-2.10.0-3-cp312-cp312-manylinux_2_28_x86_64.whl", hash = "sha256:98c01b8bb5e3240426dcde1446eed6f40c778091c8544767ef1168fc663a05a6", size = 915622781, upload-time = "2026-03-11T14:17:11.354Z" }, + { url = "https://files.pythonhosted.org/packages/ab/c6/4dfe238342ffdcec5aef1c96c457548762d33c40b45a1ab7033bb26d2ff2/torch-2.10.0-3-cp313-cp313-manylinux_2_28_x86_64.whl", hash = "sha256:80b1b5bfe38eb0e9f5ff09f206dcac0a87aadd084230d4a36eea5ec5232c115b", size = 915627275, upload-time = "2026-03-11T14:16:11.325Z" }, + { url = "https://files.pythonhosted.org/packages/d8/f0/72bf18847f58f877a6a8acf60614b14935e2f156d942483af1ffc081aea0/torch-2.10.0-3-cp313-cp313t-manylinux_2_28_x86_64.whl", hash = "sha256:46b3574d93a2a8134b3f5475cfb98e2eb46771794c57015f6ad1fb795ec25e49", size = 915523474, upload-time = "2026-03-11T14:17:44.422Z" }, + { url = "https://files.pythonhosted.org/packages/f4/39/590742415c3030551944edc2ddc273ea1fdfe8ffb2780992e824f1ebee98/torch-2.10.0-3-cp314-cp314-manylinux_2_28_x86_64.whl", hash = "sha256:b1d5e2aba4eb7f8e87fbe04f86442887f9167a35f092afe4c237dfcaaef6e328", size = 915632474, upload-time = "2026-03-11T14:15:13.666Z" }, + { url = "https://files.pythonhosted.org/packages/b6/8e/34949484f764dde5b222b7fe3fede43e4a6f0da9d7f8c370bb617d629ee2/torch-2.10.0-3-cp314-cp314t-manylinux_2_28_x86_64.whl", hash = "sha256:0228d20b06701c05a8f978357f657817a4a63984b0c90745def81c18aedfa591", size = 915523882, upload-time = "2026-03-11T14:14:46.311Z" }, { url = "https://files.pythonhosted.org/packages/cc/af/758e242e9102e9988969b5e621d41f36b8f258bb4a099109b7a4b4b50ea4/torch-2.10.0-cp312-cp312-manylinux_2_28_aarch64.whl", hash = "sha256:5fd4117d89ffd47e3dcc71e71a22efac24828ad781c7e46aaaf56bf7f2796acf", size = 145996088, upload-time = "2026-01-21T16:24:44.171Z" }, { url = "https://files.pythonhosted.org/packages/23/8e/3c74db5e53bff7ed9e34c8123e6a8bfef718b2450c35eefab85bb4a7e270/torch-2.10.0-cp312-cp312-manylinux_2_28_x86_64.whl", hash = "sha256:787124e7db3b379d4f1ed54dd12ae7c741c16a4d29b49c0226a89bea50923ffb", size = 915711952, upload-time = "2026-01-21T16:23:53.503Z" }, { url = "https://files.pythonhosted.org/packages/6e/01/624c4324ca01f66ae4c7cd1b74eb16fb52596dce66dbe51eff95ef9e7a4c/torch-2.10.0-cp312-cp312-win_amd64.whl", hash = "sha256:2c66c61f44c5f903046cc696d088e21062644cbe541c7f1c4eaae88b2ad23547", size = 113757972, upload-time = "2026-01-21T16:24:39.516Z" },