fix(integrations): renumber INTEGRATION_SPECS to resolve order collis…#2791
Conversation
…ions Pre-existing main carried 9 collisions surfaced by the registry contract tests (Tracer-Cloud#2773): 4 setup_order ties and 5 verify_order ties. Renumber the later-added spec in each collision pair (preserving first-occurrence in source = original number) to a free slot at the high end of the range, with one fill-in at verify_order=6 (the only existing gap): setup_order tracer 12->25, telegram 19->26, whatsapp 19->27, mysql 20->28, dagster 24->29 verify_order victoria_logs 2->6, kafka 22->37, mysql 27->38, openclaw 28->39, dagster 36->40 The two xfail markers on test_setup_orders_are_unique and test_verify_orders_are_unique are now removed since both assertions pass on this state. Any future PR that reintroduces a collision will fail those tests strictly. Closes Tracer-Cloud#2790
Greptile code reviewThis repo uses Greptile for automated review. Before merge, aim for Confidence Score: 5/5 with zero unresolved review threads — see CONTRIBUTING.md. Run a review — add a PR comment with: Give it ~5-10 minutes (sometimes longer) for results, then fix feedback and re-trigger until you reach Confidence Score: 5/5. Optional: automate with the greploop skill. |
Greptile SummaryResolves 9 pre-existing
Confidence Score: 5/5Safe to merge — all changes are mechanical integer renumbering with no logic alterations, and the uniqueness tests now strictly enforce the invariant. Every collision listed in the PR table is resolved in the diff, no new collisions are introduced, and the two contract tests are converted from xfail to strict, so any future regression will immediately fail CI. The only side-effect of the renumbering is a shift in wizard/verify listing positions for the affected services, which is the intended outcome. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[INTEGRATION_SPECS tuple] --> B{setup_order set?}
A --> C{verify_order set?}
B -- yes --> D[SUPPORTED_SETUP_SERVICES\nsorted by setup_order]
C -- yes --> E[SUPPORTED_VERIFY_SERVICES\nsorted by verify_order]
D --> F[Wizard step ordering]
E --> G[integrations verify listing order]
H[test_setup_orders_are_unique\n✅ now strictly enforced] --> D
I[test_verify_orders_are_unique\n✅ now strictly enforced] --> E
subgraph "Renumbered specs (this PR)"
J[tracer 12→25]
K[telegram 19→26]
L[whatsapp 19→27]
M[mysql 20→28]
N[dagster 24→29]
O[victoria_logs 2→6]
P[kafka 22→37]
Q[mysql 27→38]
R[openclaw 28→39]
S[dagster 36→40]
end
J & K & L & M & N --> D
O & P & Q & R & S --> E
Reviews (1): Last reviewed commit: "fix(integrations): renumber INTEGRATION_..." | Re-trigger Greptile |
|
@muddlebee @Devesh36 kindly review |
Hey @Davidson3556 can we keep such type of issue for community ? |
|
LG 👍 |
|
🎻 "The diff was clean, the tests did pass, the reviewer wept." That poem was about @Davidson3556's PR. 🥹
👋 Join us on Discord - OpenSRE : hang out, contribute, or hunt for features and issues. Everyone's welcome. |
Fixes #2790
Describe the changes you have made in this PR -
app/integrations/registry.pycarried 9 pre-existing order collisions surfaced by the registry contract tests merged in #2773 (4setup_orderties and 5verify_orderties). The two xfail markers ontest_setup_orders_are_uniqueandtest_verify_orders_are_uniquewere placed exactly to track this.Renumber the later-added spec in each collision pair to a free slot at the high end of the range, with one fill-in at
verify_order=6(the only existing gap in the 0–36 range). The earlier-occurring spec in source keeps its original number, so the bulk of the user-visible wizard ordering is unchanged.setup_ordersetup_ordersetup_ordersetup_ordersetup_orderverify_orderverify_orderverify_orderverify_orderverify_orderThe two
@pytest.mark.xfail(strict=False)markers intests/integrations/test_registry_invariants.pyare removed along with the now-unusedimport pytest, since both tests pass on this state. Any future PR that reintroduces a collision will fail those tests strictly.Demo/Screenshot for feature changes and bug fixes -
Before (current
main, with--runxfailto surface what the contract tests catch):After this PR (normal run, no
--runxfailneeded since the markers are gone):Code Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Explain your implementation approach:
The problem is mechanical (pick non-colliding integers) but the renumber strategy matters. The wizard sort order and the
integrations verifylisting order are user-visible, so I wanted to disturb as little of the existing ordering as possible while still satisfying the uniqueness invariant.The rule I went with: in each collision pair, whichever spec appears first in the source file keeps its original number; later occurrences move to a free slot. That preserves the rough chronology of how integrations were added and avoids shuffling integrations that have shipped in a specific position for a while.
Picking the new slots, I checked which integers were already taken across the whole spec list.
setup_orderis dense from 0–24 (no gaps).verify_orderis dense from 0–36 except for6(the only free slot in that range), plus99for supabase as an outlier. So newsetup_orderslots go to 25–29, newverify_orderslots go to 37–40, and the one fill-in atverify_order=6letsvictoria_logsslot in compactly instead of pushing into the 40s.Alternatives I considered:
I went with the minimal-disruption rule because the contract tests don't care about ordering value, only ordering uniqueness; once each spec has a unique number, the registry is in spec, and future drift fails CI.
One wrinkle worth flagging: openclaw stays at
setup_order=12(it appears before tracer in source) but moves onverify_orderfrom 28 to 39 (twilio appears before openclaw in source for the verify collision). Consistent application of the source-order rule, but it does mean one spec moves on one axis and not the other.Checklist before requesting a review