Implement BatchableAnalyzerInterface for centralized batch processing

[jjroelofs]

Summary

• Implements BatchableAnalyzerInterface on AISentimentsAnalyzer with processEntity() and hasResults() methods, enabling participation in the centralized batch system from the Analyze module.
• Fixes existing bug where $context['finished'] was never set in batch operations — the centralized batch service now handles context management.
• Updates README.md and project description with batch processing documentation (admin UI path and Drush command).

Dependencies

• Depends on Add centralized batch processing infrastructure analyze#16 (centralized batch processing infrastructure in the Analyze module).

Closes #25

Test plan

• Enable the analyze_ai_sentiments module alongside analyze (with batch support from Add centralized batch processing infrastructure analyze#16).
• Navigate to /admin/config/content/analyze-batch and verify "AI Sentiments Analysis" appears as a selectable analyzer.
• Run drush analyze:batch --analyzers=analyze_ai_sentiments_analyzer --types=node:article --limit=2 and verify entities are processed.
• Run the same command again without --force and verify already-analyzed entities are skipped (processEntity returns FALSE).
• Run with --force and verify scores are deleted and regenerated.
• Verify the Analyze tab on individual content still works as before.

[jjroelofs]

Note: The drupal-check CI job will fail until dxpr/analyze#16 is merged and released, because the phpstan analysis requires BatchableAnalyzerInterface to exist in the installed drupal/analyze package. The drupal-lint check should pass independently.

[jjroelofs]

Code Review: #26

Reviewed the full diff (+58/-11 across 5 files), linked issue #25, the parent issue dxpr/analyze#15, and all sibling submodule PRs for cross-consistency.

Medium: Bug fix is implicit — old buggy code still active

Issue #25 identifies a bug: processBatch() never sets $context['finished'], breaking the progress bar. This PR fixes it by migrating to the centralized batch service, which handles context correctly. However, the old SentimentsBatchForm and SentimentsBatchService with the bug still exist and are still routable. Admins can still navigate to the module-specific batch form and hit the exact same progress bar bug.

Options:

1. Add @deprecated annotations and a deprecation notice in the form directing users to the centralized batch form.
2. Actually remove the old infrastructure (as dxpr/analyze_broken_links#2 does).
3. Fix the $context['finished'] bug in the old code as well, so both paths work.

Option 1 is the minimum for this PR. Leaving a known-buggy code path fully accessible without even a deprecation notice is a poor user experience.

Medium: Output buffering inconsistency

This PR calls renderSummary() directly without output buffering, while dxpr/analyze_ai_brand_voice#13 wraps it in ob_start()/ob_end_clean() "to prevent JSON corruption during batch." See the detailed analysis in my review of that PR. All 4 AI module PRs should be consistent.

Low: Unrelated formatting changes

SentimentsSettingsForm.php has a #tabledrag array indentation fix, and AISentimentsAnalyzer.php has a createStatusTable() indentation fix. These are unrelated to the batch migration. Prefer separate commits for formatting-only changes to keep the batch migration diff reviewable. (Same issue in the marketing_audit and security_audit PRs.)

Low: Existing batch infrastructure not deprecated

Same concern as brand_voice#13 — issue step 2 says to deprecate SentimentsBatchService/SentimentsBatchForm, but this PR doesn't add deprecation annotations or remove the classes. Two parallel batch systems now coexist.

Positive notes

• Core implementation is clean and follows the pattern correctly.
• hasResults() uses !empty($this->storage->getScores($entity)) (plural getScores) — correct for this module's multi-dimension score storage.
• README and project_desc updates are module-specific and well-written.
• The implicit bug fix (centralized batch service handles $context['finished']) is architecturally the right approach — the fix belongs in the infrastructure, not in each module.

[jjroelofs]

Review notes:

1. [P1] composer.json still allows drupal/analyze: ^1.1, but this PR now hard-requires Drupal\\analyze\\BatchableAnalyzerInterface from Add centralized batch processing infrastructure analyze#16. Until a tagged Analyze release containing that interface exists, Composer can still resolve the current 1.1 tag and this plugin class will fatal during discovery or enable on sites that update this module first. Please tighten the constraint to the new Analyze release line (or a temporary dev constraint while the coordinated release is pending) before merging.

[jjroelofs]

Architecture Review — Senior Drupal Perspective

The core implementation is minimal and correct (+26 lines of PHP logic). The hasResults() check using !empty($this->storage->getScores($entity)) leverages the existing content/config hash invalidation well. However, the same cross-cutting issues found in the sibling PRs apply here.

---

Critical

1. composer.json constraint allows fatal error on install

Current: "drupal/analyze": "^1.1". The BatchableAnalyzerInterface doesn't exist in any released 1.1.x tag. Sites that update this module first will get a class-not-found fatal during plugin discovery. Bump to ^1.2 or whatever version ships the interface.

Hard blocker for merge.

---

Important

2. processEntity() calls renderSummary() without output buffering — inconsistent with existing batch service

The existing SentimentsBatchService::processBatch() wraps the same call in ob_start()/ob_end_clean() specifically to "prevent JSON corruption during batch." The new processEntity() does not:

$this->renderSummary($entity);
return TRUE;

The sibling brand_voice PR does add output buffering. The other two (marketing_audit, security_audit) don't. 4 implementations, 2 different approaches — this needs consistency.

Better fix: Don't delegate through renderSummary() at all. Call analyzeSentiments() directly:

public function processEntity(EntityInterface $entity, bool $force_refresh = FALSE): bool {
  if (!$force_refresh && $this->hasResults($entity)) {
    return FALSE;
  }
  if ($force_refresh) {
    $this->storage->deleteScores($entity);
  }
  $scores = $this->analyzeSentiments($entity);
  if (!empty($scores)) {
    $this->storage->saveScores($entity, $scores);
    return TRUE;
  }
  return FALSE;
}

This avoids the output buffering question entirely, doesn't build throwaway render arrays, and correctly returns FALSE when analysis fails.

3. Old buggy batch infrastructure remains fully accessible with no deprecation

Issue #25 step 3 says "Deprecate per-module batch infrastructure." The old SentimentsBatchForm is still routable at /admin/config/analyze/sentiments/batch and still uses SentimentsBatchService::processBatch() which has the known $context['finished'] bug (progress bar never completes). Two parallel batch systems coexist — one broken, one working — with no deprecation notice.

Suggested fix: Add @deprecated annotations and @trigger_error() to both SentimentsBatchForm and SentimentsBatchService. Optionally add a messenger()->addWarning() directing users to the new centralized form.

4. processEntity() always returns TRUE — silent failures counted as successes

The method returns TRUE unconditionally after calling renderSummary(), regardless of whether analysis succeeded. If the AI provider is unconfigured, rate-limited, or returns garbage, renderSummary() handles it gracefully with status messages — but processEntity() has no way to know. The centralized batch service counts every entity as "processed."

"Processed 100/100 entities" when 50 failed silently is worse than no progress reporting at all.

---

Minor

5. README and project_desc.html deviate from prescribed copy in issue #25

Issue #25 prescribes specific "CLI & AI Agent Support" section structure. The PR uses "AI Coding Assistant Integration" with different content. The prescribed copy includes the /admin/config/content/analyze-batch admin UI path, which the PR omits — users won't discover the new centralized form.

6. Unrelated formatting changes mixed with functional changes

Array indentation fixes in SentimentsSettingsForm.php and AISentimentsAnalyzer.php are legitimate improvements but should be in a separate commit for clean attribution.

7. prepare-drupal-lint.sh pinning to dev-develop is fragile for CI

The phpcompatibility/php-compatibility:dev-develop pin is a rolling target. Consider pinning to a specific commit hash if no stable release supporting phpcs v4 exists.

---

Summary

Must fix: Bump composer constraint. Should fix: Call analyzeSentiments() directly instead of renderSummary() (fixes output buffering, failure reporting, and wasted render arrays in one shot), add deprecation annotations to old batch classes. Nice to fix: Documentation alignment.

[jjroelofs]

Live Testing Results

Tested on dxpr-cms-2026-01-07 (Drupal 11.3.1, PHP 8.3, MySQL) with all 6 modules on feature/centralized-batch-processing.

FATAL: Renderer::render() breaks CLI batch processing

Both the hasResults() path (via storage service generateContentHash() → getEntityContent()) and the processEntity() path (via renderSummary() → getHtml()) call $this->renderer->render($view). 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.

This makes the centralized batch command completely non-functional for this analyzer — zero entities can be processed.

Fix: Replace $this->renderer->render($view) with $this->renderer->renderPlain($view) in both:

1. The storage service's getEntityContent() method
2. The plugin's getHtml() method

renderPlain() creates its own render context and works correctly in CLI. After applying this fix, the batch command processes entities successfully.

After the fix, batch processing works

Tested with drush analyze:batch --analyzers=<this_analyzer> --limit=1 --force — entities are discovered, processed, and skipped correctly on subsequent runs without --force.

Rate limiting concern

When running all 4 AI analyzers concurrently (no --analyzers filter), the AI provider's rate limit is hit immediately (25+ rate limit errors in a single batch run of 13 entities). Each entity × 4 analyzers = 4 sequential API calls. No backoff or retry. Entities are counted as "processed" despite the API failures.

This is partly a base module issue (processEntity() return value not checked) and partly inherent to running multiple AI analyzers without throttling.

[jjroelofs]

One remaining concern after re-checking the latest commits: countAnalyzedEntities() is not counting distinct analyzed entities on the non-node path. It ignores the requested bundle and does a plain row count, but this table stores multiple rows per entity (one per sentiment), so drush analyze:batch --status can hit or exceed the bundle total long before every entity has actually been analyzed.

[jjroelofs]

Re-checking after the latest commits: my earlier countAnalyzedEntities() concern still stands. The new docs/render-method follow-ups didn't change the counting logic, and the non-node path still does a plain row count while this table stores multiple rows per entity (one per sentiment). That means drush analyze:batch --status can still hit or exceed the bundle total long before every entity has actually been analyzed.

[jjroelofs]

One cleanup issue still needs attention before this is really “old path removed” rather than “old path half-removed”.

analyze_ai_sentiments.links.menu.yml still registers analyze_ai_sentiments.batch, but analyze_ai_sentiments.routing.yml no longer defines that route. That leaves a dead admin menu entry pointing at a path we intentionally removed.

This is exactly the kind of leftover surface area that quietly degrades maintainability: the code looks centralized, but the UI still advertises a ghost path. Please remove the stale menu link instead of carrying dead wiring forward.