refactor issues and reporting to handle lint and build issues by bhearsum · Pull Request #3697 · mozilla/code-review

bhearsum · 2026-09-28T16:45:37Z

This is some additional refactoring I realized was needed after #3667 while working on https://bugzilla.mozilla.org/show_bug.cgi?id=2075450. Most notably, it refactors Issues and Tasks to better support the idea of builds and tests. As part of that, I discovered that there was no path to send unitresults to Phabricator at the moment, so I've changed to make that more clear, and pave the path for sending Build and Test issues (when they are created in follow-up work) as unitresults.

marco-c · 2026-10-06T13:32:39Z

The lint analysis routes are left unsuffixed to avoid confusing and change; it may be worthwhile adding additional, suffixed, routes for lints so we can get rid of the unsuffixed ones at some point?

Yes, I think it makes sense.

…lish to The lint analysis routes are left unsuffixed to avoid confusing and change; it may be worthwhile adding additional, suffixed, routes for lints so we can get rid of the unsuffixed ones at some point? In any case, we need to make sure the build/test routes don't collide with the existing ones.

The most notable implication of this is that we can now use the new IssueType to detect lint issues when publishing to phabricator. In upcoming work, a new `BuildTest` `IssueType` will be added that will allow us to do the same for those issues, and flag them as unit test results in phabricator. At the moment, there is no code path that allows unit test results to be sent.

Previously, we were using an `id` that was set incidentally by https://github.com/mozilla/code-review/blob/460c9dedaedf8a89522a3183edb152bb20bc8198/bot/code_review_bot/backend.py#L85, not a proper property of a Revision. As far as I can tell, using a stable phabricator revision or github pull request number will accomplish the same goal as using the backend revision id. This also fixes a bug in the before/after feature where the random seed was not being reset because of an early return.

bhearsum · 2026-10-06T20:28:06Z

@marco-c - I think this is ready for another pass. There's some open comments still where I think no changes are needed, but please let me know if you disagree.

I'll also highlight here that I dropped the bail-on-non-reviewbot-pushes commit because I'm going to adjust mozilla-releng/fxci-config#1247 to continue firing on code-review task completion, and find a way to do the equivalent when I add the build tasks.

We need two things here: 1) To support a new `ANALYSIS_MODE` variable when we fire code review bot via Phabricator to make a new push to try. 2) To fire the hook in response to (yet-to-be-created) build-test code review tasks completing. We also need to wait for the Phabricator build plans to set `analysis_mode` in their payloads. We'll also want mozilla/code-review#3697 to have landed, to make sure reviewbot gracefully ignores irrelevant task groups. Recent work has eliminated the need for `TRY_TASK_ID`, and `TRY_RUN_ID` has been unused since the migration to the Firefox-CI cluster AFAICT. I've removed both of these. I tested this by manually updating the `code-review-testing` hook. Both a [GENERIC_TASK_GROUP_ID](https://firefox-ci-tc.services.mozilla.com/tasks/KlsoQ1mOQQ6zBceEsMeXng) and [analysis task](https://firefox-ci-tc.services.mozilla.com/tasks/C0jvOLs5Qx-yb_iXsGqslg) fired and ran correctly; the latter with the new `ANALYSIS_MODE` variable set to `Lint` as expected.

We need two things here: 1) To support a new `ANALYSIS_MODE` variable when we fire code review bot via Phabricator to make a new push to try. 2) To fire the hook in response to (yet-to-be-created) build-test code review tasks completing. We also need to wait for the Phabricator build plans to set `analysis_mode` in their payloads. We'll also want mozilla/code-review#3697 to have landed, to make sure reviewbot gracefully ignores irrelevant task groups. Recent work has eliminated the need for `TRY_TASK_ID`, and `TRY_RUN_ID` has been unused since the migration to the Firefox-CI cluster AFAICT. I've removed both of these. I tested this by manually updating the `code-review-testing` hook. A [GENERIC_TASK_GROUP_ID](https://firefox-ci-tc.services.mozilla.com/tasks/KlsoQ1mOQQ6zBceEsMeXng), [analysis task](https://firefox-ci-tc.services.mozilla.com/tasks/LQ16KxB0TZ6HMcrxBmTWDg), and [publication task](https://firefox-ci-tc.services.mozilla.com/tasks/JFk-BwdnShmBDAAnd2RKkw) all ran correctly; the latter two with the new `ANALYSIS_MODE` set. This patch changes the bindings for both production and testing (unavoidable), but the new binding is a no-op for the moment anyways. Only the testing hook payload is being updated. I will update production separately after https://bugzilla.mozilla.org/show_bug.cgi?id=2076228 is fixed.

#1247) We need two things here: 1) To support a new `ANALYSIS_MODE` variable when we fire code review bot via Phabricator to make a new push to try. 2) To fire the hook in response to (yet-to-be-created) build-test code review tasks completing. We also need to wait for the Phabricator build plans to set `analysis_mode` in their payloads. We'll also want mozilla/code-review#3697 to have landed, to make sure reviewbot gracefully ignores irrelevant task groups. Recent work has eliminated the need for `TRY_TASK_ID`, and `TRY_RUN_ID` has been unused since the migration to the Firefox-CI cluster AFAICT. I've removed both of these. I tested this by manually updating the `code-review-testing` hook. A [GENERIC_TASK_GROUP_ID](https://firefox-ci-tc.services.mozilla.com/tasks/KlsoQ1mOQQ6zBceEsMeXng), [analysis task](https://firefox-ci-tc.services.mozilla.com/tasks/LQ16KxB0TZ6HMcrxBmTWDg), and [publication task](https://firefox-ci-tc.services.mozilla.com/tasks/JFk-BwdnShmBDAAnd2RKkw) all ran correctly; the latter two with the new `ANALYSIS_MODE` set. This patch changes the bindings for both production and testing (unavoidable), but the new binding is a no-op for the moment anyways. Only the testing hook payload is being updated. I will update production separately after https://bugzilla.mozilla.org/show_bug.cgi?id=2076228 is fixed.

bhearsum marked this pull request as ready for review September 28, 2026 16:58

bhearsum requested review from Archaeopteryx and La0 September 28, 2026 17:44

bhearsum force-pushed the fix-use-derived-persistent-ide branch from d724e9a to 1b7be3b Compare September 28, 2026 18:05

bhearsum mentioned this pull request Sep 28, 2026

add support for build-test mode reviewbot pushes in hooks mozilla-releng/fxci-config#1247

Merged

bhearsum force-pushed the fix-use-derived-persistent-ide branch from 1b7be3b to d173ca4 Compare September 28, 2026 23:36