Add centralized batch processing infrastructure

[jjroelofs]

Summary

• Add BatchableAnalyzerInterface for opt-in batch capability on analyzer plugins (follows Drupal core's ConfigurableInterface pattern)
• Add centralized AnalyzeBatchService replacing per-module batch service boilerplate (~95% duplicate code across modules)
• Add centralized AnalyzeBatchForm at /admin/config/content/analyze-batch with multi-analyzer selection
• Add drush analyze:batch command with --analyzers, --types, --limit, --force, --list options
• Update README.md and project description with batch processing documentation

Test plan

• Verify drush analyze:batch --list shows batch-capable analyzers
• Verify drush analyze:batch --analyzers=<id> --types=node:article processes entities
• Verify batch form at /admin/config/content/analyze-batch renders correctly
• Verify multi-analyzer batch runs process all selected analyzers per entity
• Verify --force flag re-processes already-analyzed entities

Resolves #15

[jjroelofs]

Code Review: #16

Reviewed the full diff (1,710+/141- across 25 files), the parent issue #15, and the three established cross-project precedents (dxpr/dxpr_theme_helper#47, dxpr/dxpr_builder#4468, dxpr/rl#32).

Critical: getFullyAnalyzedEntityIds() is O(N) entity loads

AnalyzeBatchService::getFullyAnalyzedEntityIds() loads every single entity of a type/bundle one-by-one to call hasResults() on each:

$all_ids = $query->execute();
// ...
foreach ($all_ids as $id) {
    $entity = $storage->load($id);
    // ...
    foreach ($analyzers as $analyzer) {
        if (!$analyzer->hasResults($entity)) { ... }
    }
}

On a site with 10,000 articles, this loads 10,000 full entity objects into memory just to determine which ones to exclude from batch processing. This will be catastrophically slow and memory-intensive. At minimum, use $storage->loadMultiple() in chunks. Ideally, push the existence check to a database-level query (each submodule's storage could expose a method returning analyzed entity IDs).

High: Exception in one analyzer skips remaining analyzers for that entity

In processBatch(), the try/catch wraps all analyzers for an entity:

foreach ($entities as $entity_data) {
    try {
        // ...
        foreach ($analyzers as $analyzer) {
            $analyzer->processEntity($entity, $force_refresh);
        }
    }
    catch (\Exception $e) { ... }
}

If analyzer A throws, analyzers B and C never run for that entity. The catch should be per-analyzer:

foreach ($analyzers as $id => $analyzer) {
    try {
        $analyzer->processEntity($entity, $force_refresh);
    }
    catch (\Exception $e) {
        $context['results']['errors'][] = ...;
    }
}

Medium: processEntity() return value is ignored

The interface documents @return bool TRUE if the entity was processed, FALSE if skipped but processBatch() never checks the return value. The processed counter increments per entity regardless of whether any analyzer actually did work. This makes the progress reporting misleading — "Processed 100 of 100" could mean "skipped 100 of 100."

Medium: Missing --dry-run on analyze:batch

The established cross-project pattern (dxpr_theme_helper, dxpr_builder, rl) includes --dry-run on state-changing commands. analyze:batch doesn't have this. A --dry-run that lists which entities would be processed (and by which analyzers) is valuable for verifying configuration before committing to a long-running operation.

Medium: copy() return value unchecked in installFiles()

copy($source, $dest);

If the copy fails (permissions, disk full), no error is reported. Should be:

if (!copy($source, $dest)) {
    $results[] = sprintf('Failed to copy %s', $relative);
    continue;
}

Medium: DRY violation — project root discovery

The "walk up from DRUPAL_ROOT looking for composer.json" logic is duplicated verbatim between AnalyzeCommandsBase::getProjectRoot() and analyze_requirements() in analyze.install. Extract to a static helper or trait.

Low: switchToAdmin() never calls switchBack()

Fine for CLI-only usage, but worth a @internal or code comment noting this is Drush-only. If reused from a web context, it would leave the session elevated permanently.

Low: No PHPUnit tests

Only E2E shell smoke tests exist. The batch E2E tests just check the command runs — no actual entity processing is tested. The getFullyAnalyzedEntityIds() performance issue, the per-analyzer exception handling, and the processEntity() return value semantics would all benefit from kernel tests.

Low: HTML in translatable strings

$this->t('<p>No analyzers support batch processing...</p>') — Drupal coding standards say translations should not contain HTML markup. Use #prefix/#suffix or render array wrappers instead.

Low: Form bundle list doesn't re-filter on analyzer selection

getAvailableEntityBundles() is called with ALL analyzer IDs at form build time. If a user checks only one analyzer, the bundle list still shows bundles enabled for other (unchecked) analyzers. Not blocking but potentially confusing.

Observation: Scope combines Phase 1 + Phase 2

The issue (#15) describes Phase 1 (interface, service, form, Drush batch) and Phase 2 (setup-ai, skill files, E2E, hook_requirements) as separate phases, but this PR delivers both. The PR is already large — consider whether the scope creep was intentional.

Observation: No auto-sync for SKILL.md

The other cross-project PRs include auto-sync mechanisms (schema → SKILL.md in dxpr_theme_helper, prompt.js → SKILL.md in dxpr_builder). This PR relies entirely on manual drush analyze:setup-ai. The issue mentions auto-sync but it's not implemented.

---

Summary: The interface design is clean and the overall architecture follows established patterns well. The getFullyAnalyzedEntityIds() performance issue is the most urgent fix — it will be a real problem on production sites. The per-analyzer exception handling is the second priority.

[jjroelofs]

Review notes:

1. [P1] src/Service/AnalyzeBatchService.php does all workload discovery up front. getEntitiesForAnalysis() calls getFullyAnalyzedEntityIds(), and that helper loads every entity in every selected bundle and calls hasResults() on every selected analyzer before the batch starts. src/Form/AnalyzeBatchForm.php then materializes the entire result set into one Batch API operation per 5 entities. On a site with real content volume this moves the expensive work into the initial request, defeats the point of batching, and is very likely to hit request or session limits before the first progress page renders.

2. [P1] The analyzer/type matrix is no longer enforced. getAvailableEntityBundles() includes bundles when any selected analyzer is enabled, but processBatch() runs all selected analyzers on every returned entity. In a multi-analyzer run that means Brand Voice, Sentiments, etc. can be executed against bundles where they are disabled in analyze.settings, both from the form and from CLI calls with explicit --types. The batch service needs to carry a per-bundle analyzer subset or validate the combinations before queueing work.

3. [Collection/P1] The five dependent module PRs still allow drupal/analyze: ^1.1 in composer.json, so this series is not safely releasable as-is. Any site that updates one of those modules before an Analyze release containing BatchableAnalyzerInterface exists can still resolve the current 1.1 tag and hit a fatal interface mismatch during plugin discovery.

[jjroelofs]

Architecture Review — Senior Drupal Perspective

I've reviewed this PR in the context of issue #15 and the 5 dependent submodule PRs. The interface design (BatchableAnalyzerInterface) is clean and follows Drupal's established opt-in interface pattern well. The overall architecture is sound. However, several issues need attention before merge.

---

Critical

1. getFullyAnalyzedEntityIds() is O(N) entity loads — will OOM/timeout on production sites

In AnalyzeBatchService.php, getFullyAnalyzedEntityIds() loads every single entity one-by-one to check hasResults():

foreach ($all_ids as $id) {
    $entity = $storage->load($id);
    foreach ($analyzers as $analyzer) {
        if (!$analyzer->hasResults($entity)) { ... }
    }
}

On a site with 10,000 articles and 3 analyzers, that's 10,000 individual $storage->load() calls before batch processing even starts. This defeats the purpose of batching — the heavy work is front-loaded into the initial HTTP request.

Worse: in several submodules (e.g., analyze_broken_links, analyze_ai_content_security_audit), hasResults() internally calls generateContentHash() which does a full entity render. So this becomes O(N) entity renders, not just loads.

Suggested fix: Either (a) use $storage->loadMultiple() in chunks of 50, or (b) add a getAnalyzedEntityIds(string $entity_type, string $bundle): array method to BatchableAnalyzerInterface that queries the storage table directly without loading entities, or (c) push the check into the batch operation itself and let it be incremental.

2. Analyzer/bundle matrix not enforced — analyzers run against bundles where they are disabled

getAvailableEntityBundles() includes a bundle if any selected analyzer is enabled for it (due to break on first match). Then processBatch() runs all selected analyzers against every entity in those bundles:

foreach ($analyzers as $analyzer) {
    $analyzer->processEntity($entity, $force_refresh);
}

If Brand Voice is enabled for node:article and Sentiments is enabled for node:page, a multi-analyzer batch will run Brand Voice against pages and Sentiments against articles — bundles where they're explicitly disabled. This violates the admin's configuration and will cause unnecessary (potentially costly) AI API calls.

Suggested fix: Carry a per-bundle-to-analyzer mapping. Only run analyzers that are actually enabled for each entity's bundle. Alternatively, check $plugin->isEnabled($entity) inside processBatch() before calling processEntity().

---

High

3. Exception in one analyzer skips remaining analyzers for that entity

The try/catch in processBatch() wraps the entire inner loop of analyzers. If analyzer A throws on entity 42, analyzers B and C never execute for entity 42. AI-based analyzers are inherently prone to transient failures (rate limits, timeouts). One flaky analyzer should not block all others.

Fix: Move the try/catch inside the analyzer loop.

4. switchToAdmin() loads user ID 1 with no switchBack()

In AnalyzeCommandsBase.php: (a) Hardcodes user ID 1 as "admin" — user 1 may not exist on all sites. (b) Never calls $switcher->switchBack(). (c) Uses static \Drupal:: calls instead of DI.

The account switcher maintains a stack. Not calling switchBack() means the elevated session persists. If any code path triggers this from a web context (queue worker reusing this base class), it becomes a privilege escalation issue.

Fix: Inject AccountSwitcherInterface, add switchBack() in a finally block, consider UserSession with admin role instead of loading user 1.

5. processEntity() return value silently ignored

BatchableAnalyzerInterface::processEntity() returns bool (TRUE = processed, FALSE = skipped), but the return value is never checked. The processed counter increments unconditionally. Progress reporting becomes misleading — "Processed 1000 entities" could mean "skipped 1000."

Fix: Track both processed and skipped counts separately. Report them in the finish callback.

---

Medium

6. $context['finished'] calculation can cause infinite batch loop

$context['finished'] = $context['results']['processed'] / $context['sandbox']['total_entities'];

If entities fail to load (NULL from $storage->load()), processed never reaches total_entities, and finished never reaches 1.0. The Batch API will keep calling the operation forever.

Fix: Track an attempted counter that increments regardless of success, and use that for the finished calculation.

7. No --dry-run on analyze:batch

On a large site, running drush analyze:batch without previewing could trigger thousands of API calls to external AI services, incurring significant costs. The cross-project pattern from dxpr_theme_helper, dxpr_builder, and rl all include --dry-run on state-changing commands.

8. Unchecked copy() and mkdir() in installFiles()

In AnalyzeSetupCommands.php, both copy() and mkdir() return values are unchecked. If either fails (permissions, disk full), the command reports success. Silent failure.

9. DRY violation — project root discovery duplicated

AnalyzeCommandsBase::getProjectRoot() and analyze_requirements() in analyze.install contain identical logic for finding the project root via composer.json traversal. Extract to a shared utility.

10. HTML in translatable strings

In AnalyzeBatchForm.php, strings like $this->t('<p>No analyzers support batch processing...</p>') violate Drupal coding standards. HTML should be in render arrays using #prefix/#suffix, not inside t(). Translators see raw HTML, making translation error-prone.

11. Plugin instances created just to check instanceof

getBatchableAnalyzers() instantiates every Analyze plugin just to check instanceof BatchableAnalyzerInterface. Consider adding a batch flag to the plugin annotation so this check can be done at the definition level without instantiating every plugin.

---

Low

12. Form bundle list doesn't re-filter on analyzer selection

The bundle checkboxes are populated using ALL available analyzers, not the user's current checkbox selection. No AJAX callback updates the list. Selecting only one analyzer still shows bundles from other analyzers, which compounds issue #2.

13. E2E tests are smoke tests only

test-batch.sh only verifies the command runs without crashing. It doesn't install submodules, create content, or verify actual batch processing. These tests will pass even if batch processing is completely broken.

14. Scope creep: Phase 1 + Phase 2 delivered together

Issue #15 separates Phase 1 (batch infrastructure) and Phase 2 (setup-ai, skill files, E2E, hook_requirements). Delivering both in a 1,710-line PR makes review harder. Consider splitting.

15. BatchableAnalyzerInterface doesn't extend AnalyzeInterface

The new interface is standalone — any class could implement it without being an Analyze plugin. The instanceof check filters correctly, but the type system doesn't enforce the relationship. Consider either extending AnalyzeInterface or documenting why this is intentional.

---

Cross-Module Observation

All 5 submodule PRs implement processEntity() by delegating to renderSummary() — a presentation method — for its storage side effects. This is architecturally fragile: render arrays are built and immediately discarded, output buffering is inconsistently applied (brand_voice buffers, the other 3 don't), and processEntity() always returns TRUE even when analysis fails silently. The centralized batch service should ideally document this pattern expectation, or better yet, the interface contract should encourage direct analysis+storage logic rather than render-method delegation.

[jjroelofs]

Live Testing Results — dxpr-cms-2026-01-07 (Drupal 11.3.1, PHP 8.3, MySQL)

All 6 modules on feature/centralized-batch-processing branch, symlinked into a production-like DXPR CMS site with 14 content nodes (5 blog, 8 landing_page, 1 unpublished page).

FATAL — Command completely broken out of the box

F1. $next_offset never initialized → TypeError on every invocation

TypeError: AnalyzeBatchService::collectProcessableEntities(): Argument #7 ($next_offset) 
must be of type int, null given

Reproduction: drush analyze:batch (any arguments). The variable $next_offset is used at line 193 but never initialized before the while loop at line 181. This affects all three code paths: runCliBatch(), getEntitiesForAnalysis(), and processBundleBatch().

Fix: Add $next_offset = 0; before each while loop that passes it by reference to collectProcessableEntities(). Three locations need fixing.

Impact: The entire batch command is non-functional. Zero entities can be processed.

F2. Renderer::render() called outside render context → LogicException in CLI

After fixing F1, every hasResults() call fails because the storage services call $this->renderer->render($view) to compute content hashes. In Drush CLI context, there's no render root, so Drupal throws:

LogicException: Render context is empty, because render() was called outside of a 
renderRoot() or renderPlain() call.

Affected files (in storage services — hasResults() path):

• analyze_ai_brand_voice/src/Service/BrandVoiceStorageService.php:178
• analyze_ai_sentiments/src/Service/SentimentsStorageService.php:213
• analyze_ai_content_security_audit/src/Service/SecurityVectorStorageService.php:294

Affected files (in plugins — processEntity() → renderSummary() → getHtml() path):

• analyze_ai_brand_voice/src/Plugin/Analyze/AIBrandVoiceAnalyzer.php:258
• analyze_ai_sentiments/src/Plugin/Analyze/AISentimentsAnalyzer.php:399
• analyze_ai_content_security_audit/src/Plugin/Analyze/AIContentSecurityAuditAnalyzer.php:402
• analyze_ai_content_marketing_audit/src/Plugin/Analyze/AIContentMarketingAuditAnalyzer.php:791

Fix: Replace $this->renderer->render($view) with $this->renderer->renderPlain($view) in all 7 locations. renderPlain() creates its own render context and works in CLI.

Impact: After fixing F1, the batch command still can't process ANY entity across ANY analyzer.

---

After applying both fixes, the command works. Further findings:

PASS ✓

Test	Result
analyze:batch --list	Lists all 5 batch-capable analyzers correctly
analyze:batch --limit=2	Discovers entities, paginates correctly, processes exactly 2
analyze:batch --force --limit=2	Re-processes already-analyzed entities
analyze:batch --analyzers=analyze_ai_brand_voice_analyzer --limit=1	Filters to single analyzer
analyze:batch --types=node:landing_page --limit=1 --force	Filters to specific bundle
analyze:batch --types=node:page → "No entities found"	Correctly skips unpublished content
analyze:batch --analyzers=nonexistent	Graceful error: "Unknown analyzer"
analyze:batch --types=node:nonexistent	Graceful warning: "No selected analyzers are enabled"
analyze:batch --types=invalidformat	Graceful warning, no crash
analyze:setup-ai	Installs all 3 skill files correctly
analyze:setup-ai --check	Reports "up to date" after install, "NOT INSTALLED" before
analyze:setup-ai --host=claude	Installs only Claude skill file
analyze:setup-ai --host=agents	Installs only agents skill files
analyze:broken-links:report	Works correctly after enabling module
Batch form route /admin/config/content/analyze-batch	Route exists and form class is loadable

FAIL / ISSUES

Test	Issue
Full batch run (all analyzers, no limit)	AI rate limiting floods with no backoff — 25+ rate limit errors logged. All entities counted as "processed" despite failures
analyze:broken-links:scan	Command deleted but analyze:broken-links:report still references it in "no data" message
core:requirements --filter=analyze after deleting skill files	No warning shown — analyze_requirements() only checks for stale files (hash mismatch), not missing files
Old per-module batch routes	All 4 old batch routes still accessible at /admin/config/analyze/{module}/batch with no deprecation notice
Drupal 11.3 deprecation	Calling Renderer::render with NULL is deprecated in drupal:11.3.0 logged during batch processing

Summary

The analyze:batch command is completely non-functional as shipped due to two fatal bugs (uninitialized variable + render context). After patching those 10 lines across 8 files, the core functionality works well — entity discovery, pagination, filtering, and progress tracking all behave correctly. The analyze:setup-ai command works flawlessly.

The processEntity() → renderSummary() delegation pattern is the root cause of the render context issue and should be reconsidered for CLI/batch contexts.

[jjroelofs]

Two remaining bugs after the latest follow-up commits:

• AnalyzeBatchService::processBatch() still ignores the boolean returned by processEntity(). The new analyzers now return FALSE when analysis fails or produces no result, but the batch loop treats any non-exception path as success, so the per-entity OK/FAILED output and final success counts can still over-report success.

• getAnalysisStatus() uses min(countAnalyzedEntities(...)) as the count of entities analyzed by all selected analyzers. That only works if every analyzer has results on the same entity set. If analyzer A has results on one subset and analyzer B on a different subset, --status can report non-zero coverage even when the true intersection is 0.

[jjroelofs]

Re-checking after the latest follow-up commits, I still don't think the batch/status logic is fully addressed:

• processBatch() still treats any non-exception analyzer run as success and ignores the boolean returned by processEntity(), so analyzers that now return FALSE on analysis failure/no-result can still be counted as OK.
• The original analyzer-to-bundle matrix bug also still looks present: bundles are selected when any chosen analyzer is enabled on them, but the batch loop then runs all selected analyzers on every entity in that bundle.
• getAnalysisStatus() still uses min(countAnalyzedEntities(...)) as if it were the intersection of analyzed entities across analyzers. That is only valid when every analyzer has results on the same entity set.

[jjroelofs]

There is still dead duplication in the new cleanup work.

src/ProjectRootHelper.php was added, but nothing uses it; AnalyzeCommandsBase still keeps its own inline getProjectRoot() implementation, and getModulePath() also appears to be unused. Adding a new helper without deleting or adopting the old path is exactly how codebases accumulate permanent baggage.

I want to push back pretty hard on this kind of drift: every extra helper and every unused path increases maintenance cost for zero product value. Please either wire the new helper in and delete the superseded code, or delete the helper. We should not merge redundant abstractions into a codebase that is explicitly trying to get smaller and cleaner.