fix: offload bulk audio generation off the request path#573
Merged
Conversation
Bulk "Generate audio" looped one blocking 30s API call per selected post inside the edit.php redirect request — an unbounded N+1 that violates VIP (blocking remote calls on a request path) and can kill the request mid-loop off-VIP. handle_bulk_generate_action() now delegates to a new Sync::bulk_generate_audio_for_posts(), split by environment: - On VIP (async enabled), flag each post and schedule one background cron job per post via the existing Cron Control path, so the request returns immediately with no blocking calls. - Off-VIP (no reliable WP-Cron), stay synchronous but hard-cap the batch (beyondwords_bulk_generate_sync_limit, default 10), lower the per-call timeout to 15s, process posts still missing audio first, and defer the overflow behind a new admin notice. The API timeout now comes from the beyondwords_api_request_timeout filter instead of a literal, which is phpcs-clean under the VIP RemoteRequestTimeout sniff and retires the previous phpcs:ignore. No scheduled tasks are used off-VIP; cron stays VIP-only via the existing is_async_generation_enabled() gate. Verified: phpcs (WordPress-VIP-Go) clean; PHPUnit green including 8 new tests plus the BulkEdit integration tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
✅ WordPress Plugin Check Report
📊 ReportAll checks passed! No errors or warnings found. 🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check |
…1a3f Reconciles the bulk-generate offload with main's sibling change 2be1ca0 ("defer audio deletion to VIP cron and shorten DELETE timeout"), which solved the same class of problem for DELETE and landed first. Both branches removed the phpcs:ignore on the request timeout but by different means: main parametrized Client::build_args()/call_api() with an explicit $timeout (REQUEST_TIMEOUT / DELETE_TIMEOUT), while this branch routed it through a beyondwords_api_request_timeout filter. Main's approach is now trunk, so the filter is dropped and the bulk path adopts the parameter: - create_audio() and update_audio() take an optional $timeout (defaulting to Client::REQUEST_TIMEOUT), which main had only done for the DELETE path. - Sync::generate_audio_for_post() and update_or_recreate_audio() forward a $timeout, so the capped synchronous bulk batch passes BULK_GENERATE_TIMEOUT instead of scoping a global filter. The Sync conflicts were additive and both sides are kept: main's DELETE_AUDIO_CRON_HOOK and delete_audio_by_ids() alongside this branch's BULK_GENERATE_* constants and bulk_generate_audio_for_posts(). Verified: full phpcs ruleset clean; full PHPUnit suite passes (663 tests, 2141 assertions, 1 multisite-only skip). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
galbus
marked this pull request as ready for review
July 19, 2026 07:33
…1a3f Integrates main's security fix ff6f5e8 ("enforce per-post edit_post capability in bulk audio handlers") with this branch's rewrite of handle_bulk_generate_action(). Main added the per-post capability filter to the original inline generate loop, which this branch had replaced with a delegation to Sync::bulk_generate_audio_for_posts(). Resolved by keeping main's capability gate and empty-selection bail ahead of the delegation, so the security check still runs before any audio is generated. Main's sort() and counter init are dropped because Sync now normalises, dedupes and sorts the IDs itself. The test file conflicts were both sides adding tests, so main's version is taken wholesale and this branch's two cases are re-appended. handle_bulk_generate_action_defers_beyond_sync_cap() now runs as an administrator, matching how main fixed its own integration tests: the new capability gate rejects user 0, which would otherwise empty the selection and skip the cap/defer path under test. Verified: full phpcs ruleset clean; full PHPUnit suite passes (667 tests, 2159 assertions, 1 multisite-only skip), including all 15 BulkEdit cases from both branches. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…1a3f Only src/post/class-sync.php conflicted, and additively: main's ffedee9 ("stop register_meta leaking secret post meta over REST") added the REST_LEGACY_META_KEYS and REST_PRIVATE_META_KEYS constants in the same block where this branch added BULK_GENERATE_SYNC_LIMIT and BULK_GENERATE_TIMEOUT. Both sides are kept, grouped as cron hooks, then bulk-generation tuning, then REST meta registration. src/api/class-client.php merged cleanly: main's new RENDER_TIMEOUT (3s) and VOICES_TIMEOUT (8s) sit alongside the $timeout parameters this branch added to create_audio()/update_audio(). REQUEST_TIMEOUT is still 30s, so BULK_GENERATE_TIMEOUT (15s) remains correctly bounded between DELETE_TIMEOUT and the default. Verified: full phpcs ruleset clean; full PHPUnit suite passes (699 tests, 2229 assertions, 1 multisite-only skip). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
galbus
marked this pull request as draft
July 19, 2026 08:12
Main's #582 collapsed the timeout constants: REQUEST_TIMEOUT (30s), DELETE_TIMEOUT and RENDER_TIMEOUT are gone, replaced by Client::DEFAULT_REQUEST_TIMEOUT (3s), with VOICES_REQUEST_TIMEOUT (8s) the only exception. Git auto-merged this textually but the result was semantically broken: create_audio(), update_audio(), generate_audio_for_post() and update_or_recreate_audio() all still defaulted their $timeout parameter to the deleted Client::REQUEST_TIMEOUT, which fatals at call time, and a SyncTest assertion referenced it too. BULK_GENERATE_TIMEOUT (15s) existed only to pull the bulk loop below the old generous 30s default. With the default now 3s it would instead make the bulk path five times more permissive than every other call — the opposite of this branch's intent — so it is removed entirely, along with the $timeout parameter it was threaded through. Those four signatures return to matching main, and the bulk loop inherits the 3s default. BULK_GENERATE_SYNC_LIMIT stays: it bounds the number of blocking calls, which is the actual N+1 fix and is orthogonal to per-call duration. The shorter default makes it strictly better — worst case drops from 10 x 15s = 150s to 10 x 3s = 30s, inside a typical execution limit. The SyncTest assertion is rewritten to guard exactly that invariant (cap x per-call timeout <= 60s) instead of the removed constant. Verified: full phpcs ruleset clean; full PHPUnit suite passes (703 tests, 2240 assertions, 1 multisite-only skip). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The synchronous bulk-generate cap was filterable, but the plugin is reducing its filter surface and this one earned its keep mainly as test scaffolding. BULK_GENERATE_SYNC_LIMIT is now read directly, and the bulk_generate_sync_limit() wrapper — which existed only to apply the filter and guard a non-positive override — is removed with it. This PR now introduces no new filters at all. Both cap tests previously shrank the limit via that filter. They now select BULK_GENERATE_SYNC_LIMIT + 2 posts and derive every expectation from the constant, so they exercise the real cap and keep passing if it is ever retuned. The Sync test also gains an assertion that the already-generated post is ordered last and so is genuinely deferred with its content ID untouched, which the old tiny-cap version could not show. Verified: full phpcs ruleset clean; full PHPUnit suite passes (703 tests, 2258 assertions, 1 multisite-only skip). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
galbus
marked this pull request as ready for review
July 19, 2026 09:59
gouravkhunger
approved these changes
Jul 19, 2026
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
2 participants
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.
Problem
The bulk Generate audio action made one blocking remote API call per
selected post, inline in the
edit.phpredirect request(
handle_bulk_generate_action()→Sync::generate_audio_for_post()→Client::create_audio()/update_audio()). With Screen Options allowing upto 999 rows plus select-all, that is an unbounded N+1 HTTP loop in a single
admin request:
request path.
fires, the user sees a blank/timeout page and no notice, and re-running
duplicates API work.
Unlike the delete path (one batched
/content/batch_delete), generate hadno batching and never consulted the existing
is_async_generation_enabled()cron-offload path — so even on VIP the loop ran inline.
Fix
handle_bulk_generate_action()now delegates to a newSync::bulk_generate_audio_for_posts(), which dispatches by environmentusing the existing
is_async_generation_enabled()gate:beyondwords_generate_audiometa + schedule one background cron job per post via the existing Cron Control path. Request returns immediately — zero blocking calls.Non-VIP uses no scheduled tasks
Cron is used only on VIP (unchanged
is_async_generation_enabled()behaviour), because WP-Cron is unreliable off-VIP. Off-VIP hosts get the
capped-synchronous path instead — this PR introduces no new WP-Cron
dependency on non-VIP. Hosts that later have reliable cron can opt in via
the existing
beyondwords_async_generate_audiofilter.Relationship to #582 (already merged)
An earlier revision of this PR added its own
BULK_GENERATE_TIMEOUT(15s)to pull the bulk loop below the then-30s default. #582 has since collapsed
the timeout constants to
Client::DEFAULT_REQUEST_TIMEOUT(3s), so thatconstant was removed — at 15s it would have made the bulk path more
permissive than every other call. The bulk loop now simply inherits the
shared 3s default, and
src/api/class-client.phpis untouched by this PR.That makes the cap the thing that bounds the total, and it bounds it well:
worst case is now 10 × 3s = 30s, inside a typical PHP/VIP execution
limit (it was 10 × 15s = 150s). A regression test asserts exactly this
invariant, so raising either the cap or the shared timeout into an unsafe
combination fails CI.
Reviewer notes
edit_postcapability gate from fix: enforce per-post edit_post capability in bulk audio handlers #572 (ff6f5e8) ispreserved, and runs before the delegation — no audio can be generated
for posts the user cannot edit.
Syncnormalises/dedupes/sorts the IDs,so the handler's old
sort()is gone.shows "X posts still need audio — run Generate audio again to
continue." This is the deliberate trade-off of not scheduling off-VIP.
Deferred posts are ordered missing-audio-first so re-running converges
rather than re-updating the same already-done posts.
wp_next_scheduled),consistent with the single-save async path.
beyondwords_bulk_deferredis added to both bulk handlers' query-argcleanup lists so a stale notice can't linger across actions.
Verification
src/tree): clean, exit 0, nophpcs:ignoreadded.703 tests, 2258 assertions, 1 multisite-only skip. Includes 8 new
test cases: VIP schedule path, non-VIP cap/defer, ID normalisation, the
worst-case timeout-budget guard, the deferred notice (+ nonce handling),
and the redirect deferred-count.
posts-listspec (bulk-actions.cy.js) passed on the last full CI run.