Skip to content

fix(theme): serialize Polylang group linking in AI media translation#150

Merged
JohnRDOrazio merged 2 commits into
mainfrom
fix/cdcf-media-translation-link-race
May 25, 2026
Merged

fix(theme): serialize Polylang group linking in AI media translation#150
JohnRDOrazio merged 2 commits into
mainfrom
fix/cdcf-media-translation-link-race

Conversation

@JohnRDOrazio
Copy link
Copy Markdown
Member

@JohnRDOrazio JohnRDOrazio commented May 25, 2026

Root cause

The translation-group asymmetry we hit on imported team-member photos (Karačić: IT+FR orphaned; Bersano Calot: IT orphaned) is a lost-update race, not an MCP issue.

Media translations run through the theme's "AI Translation" feature (edit media → Translate). The "Translate All" button fires one concurrent AJAX request per language (functions.php, Promise.all), and the handler linked the Polylang group with a non-atomic read-modify-write of the single shared post_translations term:

$translations = pll_get_post_translations($source_id); // READ group
$translations[$target_lang] = $post_id;                // add only THIS language
pll_save_post_translations($translations);             // WRITE shared term

Served by parallel PHP-FPM workers, each request reads the group before its siblings commit, adds only its own language, and overwrites the shared term — so the later writers clobber the earlier ones and one or two languages end up split into a stale term. Timing-dependent, which is why the orphaned language differed between authors.

The MCP/queue create-* handlers are immune: they build the full translations array and save it once, sequentially, in a single request.

Fix

Wrap only the linking critical section in a MySQL GET_LOCK() advisory lock keyed on the source group id (cdcf_pll_link_<source_id>). The concurrent requests now queue for the ~millisecond link, then run their expensive OpenAI translations fully in parallel.

This deliberately keeps the concurrency: a client-side sequential fix would also serialize the slow OpenAI calls, losing the speedup that the parallel design (and the Redis workers) exist to provide. Only the cheap group-linking is serialized.

Lock acquisition is best-effort — if $wpdb is unavailable or the lock times out (10s; the section is sub-millisecond), linking still proceeds rather than failing the translation.

Tests

  • test_autocreate_links_group_under_a_source_keyed_lock — fake $wpdb asserts GET_LOCK/RELEASE_LOCK are called keyed on the source id, and the group is saved with source+target merged.
  • test_autocreate_still_links_when_no_wpdb_available — degraded path still links.
  • Full theme suite: 370 green. Patch coverage 100% (new helper + call line); the 4 uncovered lines reported in the file are pre-existing and outside this diff.

Note

This fixes future translations. The two already-imported authors were repaired manually (their groups are now symmetric).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced translation linking to better handle concurrent translation operations, ensuring reliable synchronization when multiple translations are processed simultaneously.
  • Tests

    • Added test coverage for translation linking behavior under various operational conditions.

Review Change Stack

The "Translate All" button on the media/post edit screen fans out one
concurrent AJAX request per language (Promise.all), and each request did a
non-atomic read-modify-write of the shared Polylang post_translations term
(read source group -> add this language -> save). Served by parallel
PHP-FPM workers, the requests lost-update each other's group term, leaving
one or two languages orphaned in a stale term — the translation-group
asymmetry seen on imported team-member photos (IT/FR for one author, IT for
another; the orphaned language varied by timing).

Wrap only the linking critical section in a MySQL GET_LOCK advisory lock
keyed on the source group id. The concurrent requests now queue for the
~millisecond link, then run their expensive OpenAI translations fully in
parallel — so the speedup that motivated concurrency is preserved (unlike a
client-side sequential fix, which would serialize the slow OpenAI calls
too). Lock acquisition is best-effort: if $wpdb is unavailable or the lock
times out, linking still proceeds rather than failing the translation.

The MCP/queue create-* handlers were never affected: they build the full
translations array and save it once, sequentially, in a single request.

Tests: assert the lock is acquired+released keyed on the source id and the
group is saved with source+target merged; plus a degraded-path test (no
$wpdb) confirming linking still completes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 25, 2026

Warning

Review limit reached

@JohnRDOrazio, we couldn't start this review because you've used your available PR reviews for now.

Your plan includes 1 review of capacity. Refill in 22 minutes and 40 seconds.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more review capacity refills, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 01fe78ef-a14e-4fd5-9dda-4cd5f3210863

📥 Commits

Reviewing files that changed from the base of the PR and between 4786931 and efab61d.

📒 Files selected for processing (2)
  • wordpress/themes/cdcf-headless/includes/admin/ai-translate.php
  • wordpress/themes/cdcf-headless/tests/AjaxAiTranslateTest.php
📝 Walkthrough

Walkthrough

The PR adds MySQL advisory lock serialization to Polylang translation-group linking in the AJAX autocreate flow. A new cdcf_ai_translate_link_under_lock() helper acquires a source-keyed lock, performs the Polylang read-modify-write, and releases the lock in a finally block. The AJAX handler integrates this function, and test coverage validates both lock-present and lock-absent paths.

Changes

Polylang translation linking serialization

Layer / File(s) Summary
Advisory lock wrapper and AJAX integration
wordpress/themes/cdcf-headless/includes/admin/ai-translate.php
cdcf_ai_translate_link_under_lock() acquires a source-keyed MySQL advisory lock, performs Polylang post_translations read-modify-write, and releases the lock in a finally block. The cdcf_ajax_ai_translate() handler calls this function to serialize translation group updates.
Lock and fallback test coverage
wordpress/themes/cdcf-headless/tests/AjaxAiTranslateTest.php
tearDown() cleanup unsets $GLOBALS['wpdb'] to prevent test interference. Two new tests verify the handler behavior: one asserts lock acquisition/release with merged translation groups when wpdb is available, and another confirms the handler still links translations via pll_save_post_translations when wpdb is unavailable.

Sequence Diagram

sequenceDiagram
  participant Handler as cdcf_ajax_ai_translate
  participant Locker as cdcf_ai_translate_link_under_lock
  participant MySQL as Database (wpdb)
  participant Polylang as Polylang API
  
  Handler->>Locker: cdcf_ai_translate_link_under_lock(source_id, target_lang, post_id)
  Locker->>MySQL: GET_LOCK(cdcf_pll_link_*source_id, timeout)
  alt Lock acquired
    MySQL-->>Locker: lock_acquired = true
    Locker->>Polylang: pll_set_post_language(post_id, target_lang)
    Locker->>Polylang: read post_translations
    Locker->>Polylang: merge new translation into group
    Locker->>Polylang: pll_save_post_translations(merged_translations)
    Locker->>MySQL: RELEASE_LOCK(cdcf_pll_link_*source_id)
    MySQL-->>Locker: released
  else Lock unavailable
    MySQL-->>Locker: lock_acquired = false
    Locker->>Polylang: pll_set_post_language(post_id, target_lang)
    Locker->>Polylang: read post_translations
    Locker->>Polylang: merge new translation into group
    Locker->>Polylang: pll_save_post_translations(merged_translations)
  end
  Locker-->>Handler: void
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A lock upon the translation thread,
MySQL guards what Polylang said,
No race conditions left to dread,
Tests confirm the path ahead! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: serializing Polylang group linking in AI media translation to fix race conditions in concurrent AJAX requests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/cdcf-media-translation-link-race

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented May 25, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 6 complexity · 0 duplication

Metric Results
Complexity 6
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@wordpress/themes/cdcf-headless/includes/admin/ai-translate.php`:
- Around line 44-50: The Polylang save/link result is ignored: after calling
pll_save_post_translations (and the surrounding
pll_set_post_language/pll_get_post_translations logic) you must check whether
the translation group was successfully saved and, if not, abort the AJAX flow;
replace the unconditional success path with a call to
cdcf_ai_translate_link_under_lock($source_id, $target_lang, (int)$post_id) (or
otherwise check the boolean return of pll_save_post_translations) and call
wp_send_json_error('Failed to link translation group.') when linking fails so
the new post is not left orphaned.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a0c2bc55-04a1-4126-b059-4804432b1350

📥 Commits

Reviewing files that changed from the base of the PR and between c4790fe and 4786931.

📒 Files selected for processing (2)
  • wordpress/themes/cdcf-headless/includes/admin/ai-translate.php
  • wordpress/themes/cdcf-headless/tests/AjaxAiTranslateTest.php

Comment thread wordpress/themes/cdcf-headless/includes/admin/ai-translate.php Outdated
Review follow-up on the linking helper: pll_save_post_translations' result
was ignored, so a real persistence failure would leave the freshly-created
translation post orphaned (created but not in any translation group) while
the AJAX flow still reported success.

cdcf_ai_translate_link_under_lock() now returns bool (false when
pll_save_post_translations returns false — the same contract used in
handlers/link-translations.php). The handler aborts on a failed link,
deleting the just-created post first so a failed attempt leaves nothing
behind, and responds wp_send_json_error('Failed to link translation group.').

Added a test for the failure path (link returns false -> orphan force-deleted
+ error). Theme suite 371 green; patch coverage 100% (the 4 uncovered lines in
the file are pre-existing and outside this diff).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants