Issue #3586192: Clean up code inconsistencies.

Ahmad Khalilrequested to merge
issue/ai_context-3586192:3586192-clean-up-ccc-inconsi into 1.0.x

Closes #3586192

Summary

Addresses the non-crossed items from #3586192 / ai-context-naming-dx-review.md, applying the reviewer's overrides from note_914050.

Changes

High

  • 1b — Delete dead AiContextItemController::revisionOverview() and orphaned getRevisionIds(). The entity.ai_context_item.version_history route uses AiContextItemRevisionController::revisionOverview().

Medium

  • 1a — Reconcile Hook/ vs Hooks/: AiSchedulerContentModerationSync is a service helper (not a #[Hook] class), so moved it to src/Service/ and updated its namespace + 3 importers. src/Hooks/ removed.
  • 1a follow-up — Removed redundant YAML registrations for DiffHooks and DynamicEntityReferenceHooks. Drupal 11's HookCollectorPass auto-registers #[Hook] classes; verified at runtime.
  • 2a — Permission schedule publishing of ai_context_items → schedule publishing of ai context items (pre-rc1, no hook_update_N needed).
  • 2b — ai_context.context_agents → ai_context.settings.agents, ai_context.context_agent_edit → ai_context.settings.agent_edit, ai_context.settings.context_agents_tab → ai_context.settings.agents_tab. All 6 callers updated (forms, controller, views field, module, twig).
  • 2c — return new static( → return new self( across 23 final classes (forms, controllers, list builder, derivative, field formatters, validator, access handler, scheduler sync). Abstract AiContextScopeBase retains new static() (correct for subclassing).
  • 2f mixed-method locals — Normalized to $snake_case in AiContextRenderer and AiContextSubcontextResolver (the worst offenders). Constructor-promoted properties remain $camelCase per Drupal convention.

Low

  • 2e — Service ID ai_context.children_service → ai_context.children (services.yml + module + list builder + kernel test).
  • 3a — AiContextUsageTracker::getContextForEntity() → getUsageRecordsForEntity() (pre-rc1 public-API rename).

Explicitly out of scope (per reviewer)

  • 3c (final policy sweep), 3d (proactive service interfaces), 3f (constructor promotion sweep) — left alone.
  • 2f codebase-wide stays Low: PHPCS Drupal.NamingConventions.ValidVariableName does not flag camelCase locals in the installed Coder version (verified with a test file). The doc's CS-violation premise is wrong; this is consistency, not compliance.
  • 2g, 2h, YAML FQCN hooks — no change (required upstream conventions).

Verification

  • phpcs --standard=Drupal,DrupalPractice src/ tests/ — clean
  • phpstan analyze on the 32 touched files — clean (15 pre-existing module-wide errors in src/Plugin/diff/Field/* and AiContextScopeLocalTaskDeriverTest are not from this MR)
  • cspell '**/*.{php,js,yml,md}' — clean
  • Test suite: 110 Unit + 253 Kernel + 92 Functional + 9 FunctionalJavascript pass. 8 Kernel errors are pre-existing infra (hardcoded web/modules/custom/ path, diff module signature drift) — not from this MR.
  • Live verification via drush php:eval: renamed routes resolve, services resolve under new IDs, renamed method exists on UsageTracker, #[Hook] classes auto-register without YAML.

Test plan

  • /admin/ai/context/settings/agents loads, lists agents, "Configure context items" hits /admin/ai/context/settings/agents/{ai_agent}/edit
  • Per-agent edit form saves and redirects back to the renamed agents route
  • Permissions UI shows "Schedule publishing of AI Context Items" with the new spaces-form machine name
  • Item revisions page renders via AiContextItemRevisionController (the kept one)
  • Deleting a parent context item clears children's parent refs (exercises ai_context.children service rename)

Merge request reports