fix(bots): ensure deterministic ordering of GitHub app installations#918
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7fcc00d. Configure here.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #918 +/- ##
=======================================
Coverage 91.89% 91.89%
=======================================
Files 1316 1316
Lines 50640 50640
Branches 1625 1625
=======================================
Hits 46538 46538
Misses 3796 3796
Partials 306 306
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| def get_github_app_installations(owner: Owner | Any): | ||
| if _is_django_model(owner): | ||
| return owner.github_app_installations.all() | ||
| return owner.github_app_installations.order_by("id") |
There was a problem hiding this comment.
shouldn't this be ordered by now based off of models/core.py? my sqlalchemy is quite weak, so just going off of what I know of django orm
There was a problem hiding this comment.
We use either orm or sqlalchemy, not together. So we are not running order on the same dataset twice, which is what I think your concern is.
thomasrockhu-codecov
left a comment
There was a problem hiding this comment.
You probably want to port this over to django ORM as well to keep consistency, but that's out of scope for this PR. I'd probably also add a test that changes the usage of the GitHubAppInstallations

Summary
DjangoSQLAlchemyOwnerWrapper.get_github_app_installationshad noORDER BY, so the DB could return installations in any order — makingapp[0]non-deterministic across callsorder_by("id")to the Django path inowner_helper.pyorder_by="GithubAppInstallation.id"to the SQLAlchemy relationship incore.pyTest plan
TestGithubAppInstallationsOrderingcreates 3 apps, deletes and recreates one (forcing its ID higher than the others), and asserts the wrapper always returns them sorted byidtest_github_apps.pytests pass🤖 Generated with Claude Code
Note
Low Risk
Low risk: adds explicit ordering to ORM queries/relationships so callers no longer depend on database return order; main impact is making GitHub app selection deterministic when lists are iterated/indexed.
Overview
Ensures deterministic ordering of
Owner.github_app_installationsacross both ORM implementations.Adds
ORDER BY idfor Django (DjangoSQLAlchemyOwnerWrapper.get_github_app_installations) and setsorder_by="GithubAppInstallation.id_"on the SQLAlchemyOwner.github_app_installationsrelationship, plus a unit test that verifies the wrapper returns installations sorted by id even after delete/recreate changes row IDs.Reviewed by Cursor Bugbot for commit 69eea4b. Bugbot is set up for automated code reviews on this repo. Configure here.