fix(governance-workflows): retry the synchronous task-complete on a deadlock - #32530
Merged
Conversation
…eadlock Resolving a workflow user task calls Flowable's taskService.complete() inline on the request thread. Under concurrent DAR-workflow writes (parallel resolves plus the async executor advancing other instances on the shared process definition), that command can lose an InnoDB deadlock race (MySQL errno 1213) while flushing its ACT_RU_* deletes — surfacing to the client as a spurious 409 on the transition. READ_COMMITTED already removes the gap-lock class; this is a record/FK-lock-ordering cycle that isolation level cannot prevent. Flowable's async executor already retries these deadlocks automatically; the synchronous resolve path was the one caller that did not. Wrap the taskService.complete() command in the existing DeadlockRetry — the same helper already guarding runtimeService.startProcessInstanceById(). The retry scope is the single self-contained Flowable command: when InnoDB rolls the transaction back it has already released every lock, so the replay runs in a fresh transaction with nothing held while it backs off. DeadlockRetry only retries a genuine deadlock (errno 1213/1205, SQLState 40001/40P01); any other failure, including a not-yet-committed task, propagates on the first attempt unchanged. Adds a DeadlockRetry.run(Runnable) overload for void commands and a unit test covering success, deadlock-then-success, non-deadlock propagation, attempt bounding, and the deadlock predicate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
github-actions
Bot
added
backend
safe to test
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, consistent with existing retry patterns in the same class, and includes meaningful unit tests for the new retry behavior and predicate.
Pull request overview
This PR makes synchronous Flowable workflow task resolution more resilient by retrying TaskService.complete(...) when MySQL/InnoDB deadlocks occur, aligning the synchronous resolve path with the already-retried async executor and existing workflow-start retry behavior.
Changes:
- Add
DeadlockRetry.run(Runnable)to support deadlock-retry for void/self-contained commands (e.g., Flowable task completion). - Wrap Flowable
taskService.complete(...)calls inWorkflowHandlerwithDeadlockRetry.run(...)for both single-approval and multi-approval paths. - Add a focused JUnit test suite covering retry behavior, deadlock detection, max-attempt bounding, and the new void overload.
File summaries
| File | Description |
|---|---|
| openmetadata-service/src/test/java/org/openmetadata/service/jdbi3/DeadlockRetryTest.java | Adds behavioral unit tests validating deadlock detection and retry semantics, including the new run(Runnable) overload. |
| openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/DeadlockRetry.java | Broadens documentation and adds run(Runnable) to support retrying void/self-contained operations. |
| openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/WorkflowHandler.java | Wraps synchronous Flowable taskService.complete(...) calls in deadlock retry to prevent intermittent 409s on deadlock. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
✅ Playwright Results — workflow succeededValidated commit ✅ 585 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 1h 0m 42s ⏱️ Max setup 4m 11s · max shard execution 17m 36s · max shard-job elapsed before upload 20m 26s · reporting 4s 🌐 212.93 requests/attempt · 2.79 app boots/UI scenario · 10.23% common-shard skew Optimization targets still in progress:
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
Code Review ✅ ApprovedWraps the synchronous OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
sonika-shah
approved these changes
Sep 3, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Sep 4, 2026
Contributor
🚦 Removed from the merge queue —
|
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Sep 5, 2026
Contributor
🚦 Removed from the merge queue —
|
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Sep 5, 2026
github-actions Bot
pushed a commit
to aokimoon/OpenMetadata
that referenced
this pull request
Sep 6, 2026
…pen-metadata#32631) * fix(playwright): root-cause four flake mechanisms behind the merge-queue ejections Triage of 22 merge-queue dequeues over 20 hours found 14 of 15 runs ejected a PR that could not have caused its failure. The failures cluster into four mechanisms, not 112 independent flaky tests. 1. Locator ambiguity, silenced with .first() `descriptionBox` is page-global, so it matches every editable block editor mounted. `updateDescription` resolved it with `.first()`, which on a page with the edit modal open selects the inline editor *behind* the overlay: visible, so the assertion passed, then the click failed on "ant-modal-wrap ... intercepts pointer events" and retried until the 60s timeout. Entity.spec "Update description" flaked in 9 of 23 sampled PRs, EntityDataSteward in 6. Adds getDescriptionBox/fillDescriptionBox to utils/common.ts, matching the API open-metadata#32599 introduces so the two resolve as duplicates rather than semantically. 97 call sites across 40 files now route through them, scoped to the real container where the code already named one, and asserting a single match everywhere else so an ambiguous page fails where the ambiguity is. utils/{domain,tag,team}.ts are left to open-metadata#32599, which already fixes them. 2. Fixture identity fixed at construction, reused across retries Entity names are generated in the constructor, and specs that build fixtures in the describe body do not re-evaluate it on a retry — so beforeAll runs again with the same names and every create answers 409. LineageFilters failed this way in 17 of 23 PRs. Eight entity classes move to the existing createOrFetch helper. Three take a `fields` argument because the by-name lookup omits what they read back (columns for File and Worksheet, dataModel for Container), checked against each resource's FIELDS rather than assumed. TaskClass is excluded (tasks are auto-numbered, nothing to conflict on), as are DashboardDataModelClass (its own 409 and 5xx handling would regress) and KnowledgeCenterClass (the /name lookup convention is unconfirmed for contextCenter pages). Two bugs surfaced on the way: DashboardClass POSTed a dashboard referencing a chart before awaiting the chart's creation, and two classes returned `response.body` — an uncalled method reference, never the entity. 3. Interacting before the page settles The glossary page keeps rendering after its loaders clear: the description block hydrates last and pushes every row down about a row height. dragTo computed both boxes, then pressed at coordinates that had moved, so no dragstart fired and the confirmation modal never opened. force: true is what let it get that far — it skips the stability check. Holds both rows still before pressing; the force option stays, the hover overlays still need it. 4. A component remounting under its own re-render TestDefinitionFormBody rebuilt `options: toOptions(Object.values(...))` on every render. Focusing a field re-renders it via onActiveFieldChange, and the new items identity made react-aria rebuild the listbox collection, detaching the option mid-click — the trace shows "element was detached from the DOM, retrying" and then nothing for the remaining 44s. The lists are enum-derived and are now built once at module scope. This one test ejected four separate PRs in 36 hours. Quarantine drops from 13 entries to 9. Four tests are released because their root cause is fixed here, each recorded in QUARANTINE.md with the diagnosis; GlossaryHierarchy "move term to root" stays, parked on a product issue rather than a flake. The count assertion changes behaviour by design: where a page really does mount two editors and .first() happened to pick the right one, the test now fails loudly instead of passing by luck. Those are latent ambiguities that each need a scope, not regressions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(playwright): restore a slow budget to the domain-rename asset test `Domain Rename Comprehensive Tests` lost its describe-scope `test.slow(true)` in open-metadata#32360, which removed all 83 blanket calls and reinstated the 26 that per-test measurements showed running over 40s of the 60s budget. Those measurements were taken from DomainAdvanced and DomainUIInteractions; this describe was not among them. `Rename domain with assets (tables, topics, dashboards) preserves associations` seeds three assets, renames the domain twice and re-verifies the associations after each rename. It has timed out at exactly 60000ms in every merge_group run since open-metadata#32360 landed, ejecting four unrelated PRs — open-metadata#32566, open-metadata#31392, open-metadata#32530 and open-metadata#32518 — across six runs. The retry gets as far as asserting the renamed header, so it is short of time, not broken. Restores the budget per-test rather than at describe scope, so the blanket ban open-metadata#32360 established still holds and the other tests in the describe keep the default. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(playwright): wait for lazy widgets, retry the edge click, drop `load` waits Three more mechanisms behind current merge-queue ejections. Lazy right-panel widgets are not covered by the loader wait The tags, glossary, owner and domain widgets are behind React.lazy, and while their chunk loads the boundary renders EntityDetailWidgetSkeleton. That is not `data-testid="loader"`, so waitForAllLoadersToDisappear returns straight past it and a test can reach for something the widgets own before they mount. `request-entity-tags` is simply absent until then, so the click waits out the whole test timeout instead of racing by a few frames — TasksUIFlow "Create and reject tag task for Dashboard" failed this way in 18 of 23 sampled PRs, the highest rate in the suite. Adds waitForWidgetsToRender and calls it from the two readiness helpers that claim the page is ready: visitEntityPageByFqn and waitForPageLoaded. Edge clicks in lineage silently no-op deleteEdge dispatches a synthetic click on a react-flow edge label, then waits for the toolbar it opens. dispatchEvent takes no actionability wait, so when the graph re-lays out between resolving the label and firing the event the click lands on a node that is no longer wired up: the toolbar never opens and the wait for `add-pipeline` burns the 300s budget on an action that did nothing. Retries the click/assert pair until the toolbar is actually there. `page.goto` defaulting to `load` on /metrics Three call sites navigated to /metrics without a waitUntil, so Playwright waited for every subresource. Under merge-queue load that exceeded the 60s navigation timeout and ejected open-metadata#32465. Each site already awaits its own readiness signal immediately afterwards — a search response, a visible table, a URL match — so they move to domcontentloaded, matching the convention used elsewhere in utils. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(playwright): consolidate description-editor resolution, remove the last .first() hangs The editor-behind-the-modal bug was not confined to `updateDescription`. Six more call sites reached for `.first()` after clicking `edit-description`, and the BlockEditorEmbedLink trace shows what that costs: a 45s click on an editor nothing could reach, twice, before the 60s timeout. Extracts the resolution that `updateDescription` was carrying inline into `resolveDescriptionBox` in utils/common.ts — prefer the editor inside the dialog when the edit opened one, assert a single match either way, retry through the modal's enter animation — and points all seven call sites at it: utils/entity.ts updateDescription (was inline) BlockEditorEmbedLink.spec.ts invalid URL validation EntityRenameConsolidation.spec.ts DataProductRenameConsolidation.spec.ts ContextCenterArticles.spec.ts text formatting / hash mention Pages/Entity.spec.ts column detail panel Two `.first()` calls stay: the deliberate fallback chain in importUtils, which tries a scoped editor before a page-wide one, and ColumnBulkOperations, which is already scoped to its drawer. om-playwright/no-positional-locator drops 11 more from the suppressions baseline (1373 -> 1362). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(playwright): return stored response data from Container/Worksheet create The createOrFetch sweep replaced the local serviceResponse/entityResponse consts with this.serviceResponseData/this.entityResponseData but left the return blocks of ContainerClass.create and WorksheetClass.create referencing the deleted locals. Every create() call threw "ReferenceError: serviceResponse is not defined" at runtime (babel transpile, no type-check), failing setup for every spec that builds a Container or Worksheet — all 27 red shards in the merge-queue run trace to this. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(playwright): give the consumer tag-asset test a slow budget 'Add and Remove Assets for Data Consumer' runs three full navigation cycles (add assets, verify filter, remove assets) and overruns the 60s default on slow shards — the two failed attempts died at different points (checkAssetsCount vs visitPage), the signature of a budget problem rather than a deterministic bug. test.slow() triples the budget for this test only, and checkAssetsCount gets the same 30s allowance the domain util already uses for the post-reload count badge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(playwright): lower the recorded no-positional-locator count to 1310 The sweep fixed 12 suppressed no-positional-locator violations and pruned eslint-suppressions.json, but the exact-count assertion in corpus.test.mjs still recorded 1322 — failing ui-checkstyle's guardrail step. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(playwright): drop unused imports left by the sweep ui-checkstyle's fixer gate (organize-imports → eslint --fix → prettier) still modified ten PR-touched files: the sweep's edits left unused imports (getDescriptionBox, descriptionBox) and unordered import blocks behind. This commit is the fixers' own output — verified a stable fixpoint locally, zero ESLint errors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ui): build the domain-type options once instead of per render `Create domains and add assets` timed out in the merge queue on a click that never landed: the Aggregate option went "not stable", then "detached from the DOM, retrying", for 169s on the first attempt and 42s on the retry. Same defect as TestDefinitionFormBody. `domainTypeOptions` was rebuilt on every render, so `Select` received a new `items` array each time and react-aria tore down and remounted the open listbox underneath the click. DomainType is an enum, so the list can never change — it moves to module scope. `dataProductTypeOptions` immediately below it was already memoized; this one was not. Swept the rest of the `options:` field bindings for the same shape. AddMetricPage and MetricExpression both build theirs inside `useMemo(..., [])`, so they were already stable and are left alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(playwright): drop the locals orphaned by the descriptionBox consolidation Routing EntityRenameConsolidation and DataProductRenameConsolidation through `resolveDescriptionBox` left each file's own `const descriptionBox = ...` unused. `tsc --project playwright/tsconfig.json` flags them as TS6133; the root `tsc --noEmit` does not, because its tsconfig only includes `src`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(playwright): resolve the editor for page-scoped fills instead of asserting Review catch. The sweep left 57 call sites on `fillDescriptionBox(page, ...)`, which asserted a single match against the whole page — so an entity page with its description modal open, two editors mounted, became a hard failure. That is the exact situation the helper exists to handle, and a test that was green because `.first()` happened to pick the right editor would have gone red. New red ejects PRs from the merge queue just as effectively as a flake does; "it was already ambiguous" explains the cause and does nothing about the effect. A `Page` scope now resolves the way a person would — the editor inside the dialog the edit just opened, falling back to the page's own — which is what `resolveDescriptionBox` already did for the six sites that used it. Only 6 of 63 sites had that behaviour; now all 63 do. An explicit `Locator` scope keeps the strict assertion: the author narrowed to that container deliberately, so two editors inside it is a real authoring bug worth surfacing rather than silently typing into one of them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(playwright): assert one editor only inside an open dialog A page legitimately hosts several description editors at once — an entity description alongside per-column ones. ColumnBulkOperations is the standing proof: it narrows to its drawer and *still* needs a first-match, so multiplicity is normal even inside a chosen container, not an anomaly. So asserting a single match against a page was wrong on its own terms, and removing the assertion alone would not have helped: `locator.fill()` is strict, so a two-match locator throws either way. The fix is to stop producing one. The bug was never "several editors on a page". It was taking the first match *while a modal was open*, which picked the inline editor behind the overlay — visible, so the assertion passed, then the click died on "ant-modal-wrap intercepts pointer events". Scoping to the dialog is the whole fix. dialog open -> the dialog's editor, and assert exactly one. This is the one place the invariant really holds; two here means the dialog selector matched something it should not have. no dialog -> first match, as it has always been. Justified disable rather than a suppression, so the baseline still only shrinks. An explicit Locator scope no longer asserts a count either; if such a container ever holds two, Playwright's own strict-mode error names the file and line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(playwright): close two more merge-queue ejection gaps Domains "Verify domain data products count includes subdomain data products" Four sequential verification steps, each with its own reload — the domain's data-product tab, the subdomain's, the tab after deleting the subdomain, and a deeply nested subdomain. The trace for run 33955229584 shows them summing to ~65s against the 60s default, cut off mid-way through the last one. Same shape as the rename test already fixed here: a genuinely slow test left without a budget when open-metadata#32360 removed the blanket ones. Per-test, not describe-scope. IngestionLogStreamLive scheduler wait The wait for a freshly triggered run to report `running` gave up at 60s while the pipeline was still `queued` — accepted by the system, just slow to start under merge-queue load (run 33970886957). That ceiling was tighter than the budget it sits inside: the `beforeAll` declares setTimeout(180_000) and spends about 6s reaching the wait, so 60s left ~114s unused. Raised to 120s, which keeps a real ceiling, still lands ~54s inside the hook's budget, and only spends headroom that already existed. A terminal state still fails fast. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(playwright): close the last two ejection gaps — delete race and grid re-sweep ExploreDiscovery "Should not display soft deleted assets in search suggestions" The test clicked Confirm on the delete modal and reloaded immediately, without waiting for the DELETE. If the server has not applied the soft delete by the time the page reloads, the table renders as live, no deleted-badge is ever mounted, and the assertion burns its full timeout. It passes locally, where the delete returns in milliseconds, and loses the race under merge-queue load. Verified: reproduced the shape locally and the fixed test passes. MetricBulkImportExportEdit "renders complex fields from listing hydration" Two things move underneath a sweep of that grid. React Data Grid virtualises columns, so a cell is only in the DOM while its column overlaps the viewport; and the complex fields — glossary terms, tags, tier — hydrate from the listing after the first paint, which is exactly what waitForMetricBulkEditGrid does not wait for: it returns once the header row and the *name* cell are up. expectVisibleAfterHorizontalScroll swept once and swallowed every miss with `.catch(() => false)`, so a cell could be missed twice over — its column not mounted when the sweep passed it, or its value not yet arrived — and the pass ended with a bare "element not found" on a cell that was merely late. It now re-sweeps until the cell appears, recomputing the scroll range each attempt because scrollWidth itself grows as more columns mount. Tag.spec "Add and Remove Assets for Data Consumer" needed nothing: it already carries a test.slow() here. The run that failed it was a merge-queue build of another PR, against main, which does not have that fix yet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(playwright): make the remaining creates retry-safe, and fix two wrong FQNs Verified against a running server this time, by calling create() twice — the shape a retried beforeAll actually produces. That caught two bugs the earlier conversions had shipped blind. DashboardClass / DashboardDataModelClass: wrong FQN in the 409 recovery DashboardDataModelRepository builds `<serviceFqn>.model.<name>` — a literal `model` segment no other entity type has. Both classes looked the conflicting entity up as `<service>.<name>`, so on a retry the 409 fallback answered 404 and the create failed anyway, with a more confusing error than the plain 409 it replaced. Strictly worse than not converting them. ContainerClass: the child container was still a raw POST Only the service and the parent container went through createOrFetch; the child kept its bare post, so a retry still 409'd. Its FQN hangs off the parent container, not the service. createOrFetch now also retries 5xx It already re-sent on 404 — the server rejected the request outright, so nothing was partially applied. A transient 5xx on create is the same case, and DashboardDataModelClass carried its own copy of that loop for exactly this reason. Folding it in lets that class drop both of its hand-rolled blocks and gives every other caller the same protection; a genuine failure still surfaces after the bounded retries. KnowledgeCenterClass needs no change: it generates page names inside create() with cryptoRandomString, so a second call cannot collide. It was on the deferred list for the wrong reason. Validated: all eight converted classes now create twice in a row cleanly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Shailesh Parmar <shailesh.parmar.webdev@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Harsh Vador <harsh.vador@somaiya.edu>
Contributor
🚦 Removed from the merge queue —
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Wraps the synchronous Flowable
taskService.complete(...)in the DAR/workflow resolve path with the existingDeadlockRetry, so a lost InnoDB deadlock race no longer surfaces as a spurious 409 on the transition.Why
Resolving a workflow user task calls
taskService.complete()inline on the request thread. Under concurrent workflow writes — parallel resolves plus the async executor advancing other instances on the shared process definition — that command can lose an InnoDB deadlock race (MySQL errno 1213) while flushing itsACT_RU_*deletes (observed ondelete from ACT_RU_IDENTITYLINK where TASK_ID_=?duringTaskServiceImpl.complete). The client sees a 409 (Workflow resolution failed ... on transition 'approve'), which shows up as intermittent CI flakes in the DAR suites.READ_COMMITTED(already configured for the Flowable MySQL engine, and verified applied at runtime) removes the gap-lock deadlock class — but this is a record/FK-lock-ordering cycle that no isolation level prevents.Flowable's async executor already retries these deadlocks automatically; the synchronous resolve path was the one caller that didn't.
runtimeService.startProcessInstanceById(...)in this same class is already wrapped inDeadlockRetry— this brings the complete/resolve path to parity.How
taskService.complete(...)command (single- and multi-approval sites) inDeadlockRetry. Scope is the one command: when InnoDB rolls the transaction back it has already released every lock, so the replay runs in a fresh transaction with nothing held while it backs off (bounded: 4 attempts, ~25–300 ms). The multi-approvalsetVariablevote commands are separate, already-committed Flowable commands outside the retry scope, so a replay cannot double-count votes.DeadlockRetryonly retries a genuine deadlock (errno 1213/1205, SQLState 40001/40P01, or the deadlock message); any other failure — including a not-yet-committed task — propagates on the first attempt, unchanged.DeadlockRetry.run(Runnable)overload for void commands; broadens the class javadoc (it now also guards self-contained Flowable commands, not only JDBI@Transactionmethods).Tests
DeadlockRetryTest(6 tests, all pass) using the production exception shape (aRuntimeExceptionwrapping aSQLExceptionwith errno 1213): success without retry, deadlock-then-success, non-deadlock propagation on attempt 1, attempt bounding at max, voidrun()replay, and theisDeadlockpredicate.Honest scope
This handles the deadlock (Flowable's own model expects retry on concurrent writes) rather than eliminating it — the underlying contention is inherent to concurrent Flowable writes on a shared process definition under InnoDB. The deadlock is intermittent and could not be reproduced deterministically off-CI, so CI across repeated runs is the real proof. The exact two-transaction lock cycle was not captured (would need
innodb_print_all_deadlocks), but every cycle in this class has the same remedy, so the fix does not depend on which pair it is.🤖 Generated with Claude Code