From 33e8c297ff1b716ef64a456b99330cbcd96edf16 Mon Sep 17 00:00:00 2001 From: joe Date: Sat, 22 Aug 2026 12:36:25 -0500 Subject: [PATCH] Add gx and gx-pro to the offline benchmark gx is an AI code review CLI. Two entries, matching the two configurations it ships with: gx full-coverage configuration gx-pro higher-precision configuration Every finding is verified before it is reported. Scored through the documented pipeline (extract, dedup, judge) on all three judge models. Core profile, the dashboard default: gx gx-pro prec recall F1 prec recall F1 sonnet-4.5 38.6% 57.0% 46.0 47.7% 46.8% 47.3 opus-4.5 41.2% 57.6% 48.0 49.7% 44.9% 47.2 gpt-5.2 34.3% 55.1% 42.2 43.6% 45.6% 44.6 The two trade coverage against precision. On the 66 Critical and High severity goldens, gx finds 43 and gx-pro 38. Both entries follow the documented "adding a new tool" flow. The benchmark PRs are forked one repo per PR per tool under the naming convention step1_download_prs.py expects, with the reviews posted on PR #1 of each: https://github.com/satoricorp-benchmarks 100 repos, 100 PRs, 383 review comments. To regenerate benchmark_data.json from scratch: uv run python -m code_review_benchmark.step1_download_prs \ --org satoricorp-benchmarks --tool gx gx posts under a user account rather than a GitHub App, so both entries are added to _NON_BOT_TOOLS alongside claude. No repository-specific configuration was used: none of the five benchmark repos carries a REVIEW.md, so every review reflects gx's defaults. Dashboard artifacts are regenerated with `python -m analysis.benchmark_dashboard`. No existing tool's metrics change; the two new entries are the only additions. --- offline/README.md | 1 + offline/analysis/benchmark_dashboard.html | 14 +- offline/analysis/benchmark_dashboard.json | 7303 ++++++++++++++--- offline/analysis/benchmark_dashboard.py | 4 + .../step1_download_prs.py | 2 +- .../candidates.json | 2588 ++++++ .../dedup_groups.json | 1317 ++- .../evaluations.json | 4882 +++++++++++ .../candidates.json | 2782 +++++++ .../dedup_groups.json | 1368 ++- .../evaluations.json | 4952 +++++++++++ offline/results/benchmark_data.json | 2993 +++++++ .../results/openai_gpt-5.2/candidates.json | 2950 +++++++ .../results/openai_gpt-5.2/dedup_groups.json | 1534 +++- .../results/openai_gpt-5.2/evaluations.json | 5152 +++++++++++- 15 files changed, 36666 insertions(+), 1176 deletions(-) diff --git a/offline/README.md b/offline/README.md index 78b2815..18c273e 100644 --- a/offline/README.md +++ b/offline/README.md @@ -19,6 +19,7 @@ Open replication of the code review benchmark used by companies like [Augment](h | [GitLab Duo](https://about.gitlab.com/gitlab-duo/) | AI code review | | [Graphite](https://graphite.dev/) | AI code review | | [Greptile](https://www.greptile.com/) | AI code review | +| [gx](https://gx.run) | AI code review | | [Propel](https://propelauth.com/) | AI code review | | [KG](https://kg.dev/) | AI code review | | [Kodus](https://kodus.io/) | AI code review | diff --git a/offline/analysis/benchmark_dashboard.html b/offline/analysis/benchmark_dashboard.html index 927d654..0aa4bff 100644 --- a/offline/analysis/benchmark_dashboard.html +++ b/offline/analysis/benchmark_dashboard.html @@ -198,15 +198,17 @@
Best for Complex Code ↗
Best for Ui ↗
Best for Critical Risk ↗
+
Best for Bug Fixes ↗
Small PRs + Performance Optimization (Precision) ↗
Bug Fixes + Cross-File ↗
Java + Authentication ↗
+
Best for Scheduling ↗
Best for Medium Python PRs ↗
Best for Concurrency ↗
-
Best for Bug Fixes ↗
Ruby + Medium PRs ↗
Highest Recall ↗
Ruby + Features ↗
+
Moderate Bugs + File Context ↗
Best for Moderate Bugs ↗
Ruby + Subtle Bugs ↗
Best for Go ↗
@@ -215,7 +217,6 @@
Best for Small PRs ↗
Best for Caching ↗
Best for Large PRs ↗
-
Best for Bug Fixes ↗
Best for Python ↗
High Risk Auth ↗
Best for File Context ↗
@@ -224,7 +225,6 @@
Best for Authentication ↗
Best for Medium Ruby PRs ↗
Best for Medium Java PRs ↗
-
Best for Scheduling ↗
Best for Typescript ↗
Best for Cross-File ↗
Best for Medium PRs ↗
@@ -428,10 +428,10 @@ ` and confirming it renders as text.", + "created_at": "2026-08-15T03:51:38Z" + }, + { + "path": "app/models/topic.rb", + "line": 719, + "body": "**Unsubscribe URL in emails is not authenticated — any recipient can unsubscribe another user**\n\nThe `unsubscribe_url` generated by `Topic#unsubscribe_url` is simply `{topic_url}/unsubscribe` with no token or HMAC. It relies solely on `current_user` (session authentication). However, the URL is placed in email bodies (`unsubscribe_link` locale). If the email is forwarded or the link shared, any logged-in user visiting it will have their own notification level changed (not necessarily the intended user's). More critically, the `List-Unsubscribe` header containing this URL may be processed by email clients that do not carry session cookies, potentially resulting in a redirect loop or error rather than the expected unsubscribe.\n\nEnsures that the unsubscribe mechanism works reliably from email clients and that authentication is handled appropriately for one-click unsubscribe flows.\n\nConsider adding a signed token (HMAC of user_id + topic_id + expiry) to the unsubscribe URL so it can work without an active session, similar to the existing `DigestUnsubscribeKey` pattern in `EmailController`. At minimum, document that the current implementation requires an active session and will not work from email clients that strip cookies. Verify by clicking the List-Unsubscribe link from an email client without an active Discourse session and confirming appropriate behavior.", + "created_at": "2026-08-15T03:51:38Z" + }, + { + "path": "app/assets/javascripts/discourse/templates/topic/unsubscribe.hbs", + "line": 3, + "body": "**XSS via triple-brace rendering of user-controlled topic title**\n\nThe Handlebars template `unsubscribe.hbs` uses `{{{stopNotificiationsText}}}` (triple braces = unescaped HTML). The computed property interpolates `model.fancyTitle` into the i18n string. `fancyTitle` may contain HTML entities from the Discourse `fancy_title` pipeline, but if a topic title contains injected HTML (admin-created or via a bug), it will be rendered unescaped in the browser.\n\nEliminates a stored XSS vector where a malicious topic title could execute JavaScript in the context of users visiting the unsubscribe page.\n\nIn `app/assets/javascripts/discourse/templates/topic/unsubscribe.hbs`, replace `{{{stopNotificiationsText}}}` with a safe approach: either use double-braces `{{stopNotificiationsText}}` and handle the bold formatting differently (e.g., separate the title into its own escaped binding), or sanitize the title before interpolation in the controller. Verify by creating a topic with a title containing `` and confirming the script does not execute on the unsubscribe page.", + "created_at": "2026-08-15T03:51:38Z" + }, + { + "path": "app/controllers/topics_controller.rb", + "line": 107, + "body": "**Handle missing TopicUser record in unsubscribe action to avoid 500s**\n\n`TopicUser.find_by(...)` can return nil (e.g., user never visited the topic or record not created yet). The code immediately dereferences `tu.notification_level`, which will raise and 500 the request. This is realistic on a link reached from email notifications (user may not have prior TopicUser row).\n\nPrevents a production 500 on a common user journey (email → unsubscribe link), improving reliability and supportability.\n\nIn `app/controllers/topics_controller.rb#unsubscribe`, replace `find_by` with a creation path when absent. For example: `tu = TopicUser.get(params[:topic_id], current_user) || TopicUser.create!(user_id: current_user.id, topic_id: params[:topic_id], notification_level: TopicUser.notification_levels[:regular])` (or use existing helper that ensures a row exists—there appears to be logic in TopicUser for ensuring consistency). Then apply the toggle and save. Verify with a controller/request spec: user with no topic_users row can hit `/t/:id/unsubscribe` and gets 200/redirect with notification_level updated/created.", + "created_at": "2026-08-15T04:15:13Z" + }, + { + "path": "app/assets/javascripts/discourse/templates/topic/unsubscribe.hbs", + "line": 4, + "body": "**Prevent HTML injection in topic unsubscribe page by removing triple-stash rendering**\n\nThe unsubscribe template renders `stopNotificiationsText` via triple-stash (`{{{...}}}`), which bypasses escaping. That text includes `{{title}}` interpolated from `model.fancyTitle`, which is derived from topic title and can be user-controlled. This is a reachable XSS path when a user clicks the unsubscribe link.\n\nEliminates a direct, user-triggerable XSS on a route that is explicitly linked from emails and thus likely to be visited from untrusted contexts.\n\nIn `app/assets/javascripts/discourse/templates/topic/unsubscribe.hbs`, change `{{{stopNotificiationsText}}}` to `{{stopNotificiationsText}}` and update `config/locales/client.en.yml` to avoid embedding raw `` tags in the translation. If bolding is required, use a safe pattern: split into separate keys and wrap the title in a `` in the template using escaped title, or use a framework-sanctioned i18n HTML helper that sanitizes interpolations. Verify by setting a topic title containing `