## Summary - remove the dead legacy drift-computation path from `CompareBaselineToTenantJob` so the strategy-driven compare engine is the only execution path left in the orchestration file - tighten compare guard and regression coverage around strategy selection, strategy execution context, findings, gaps, and no-drift outcomes - fix the repo-wide suite blockers uncovered during validation by making the governance taxonomy registry test-double compatible and aligning the capture capability guard test with current unsupported-scope behavior - add the Spec 205 planning artifacts and mark the implementation tasks complete ## Verification - `cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent` - `cd apps/platform && ./vendor/bin/sail artisan test --compact tests --stop-on-failure` - result: `3659 passed, 8 skipped (21016 assertions)` - browser smoke test passed on the Baseline Compare landing surface via the local smoke-login flow ## Notes - no Filament resource, panel, global search, destructive action, or asset registration behavior was changed - provider registration remains unchanged in `apps/platform/bootstrap/providers.php` - the compare path remains strategy-driven and Livewire v4 / Filament v5 assumptions are unchanged Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de> Reviewed-on: #235
15 KiB
Implementation Plan: Compare Job Legacy Drift Path Cleanup
Branch: 205-compare-job-cleanup | Date: 2026-04-14 | Spec: /Users/ahmeddarrazi/Documents/projects/TenantAtlas/specs/205-compare-job-cleanup/spec.md
Input: Feature specification from /Users/ahmeddarrazi/Documents/projects/TenantAtlas/specs/205-compare-job-cleanup/spec.md
Note: This plan treats Spec 205 as a mechanical closure cleanup. It removes only the dead pre-strategy drift-compute path from CompareBaselineToTenantJob, keeps the current strategy-driven compare execution unchanged, and uses focused regression plus guard coverage to prove no behavior drift.
Summary
Delete computeDrift() and its exclusive helper cluster from CompareBaselineToTenantJob, preserve the existing CompareStrategyRegistry -> IntuneCompareStrategy execution path, remove dead imports and misleading internal descriptions that survive only because of the retained legacy block, and verify unchanged behavior through focused compare execution, finding, and guard tests.
Technical Context
Language/Version: PHP 8.4.15
Primary Dependencies: Laravel 12, Filament v5, Livewire v4, Pest v4, Laravel Sail, existing BaselineCompareService, CompareBaselineToTenantJob, CompareStrategyRegistry, IntuneCompareStrategy, CurrentStateHashResolver, and current finding lifecycle services
Storage: PostgreSQL via existing baseline snapshots, baseline snapshot items, inventory items, operation_runs, findings, and current run-context JSON; no new storage planned
Testing: Pest feature and guard tests run through Laravel Sail, with focused compare execution and file-content guard coverage
Target Platform: Laravel web application under apps/platform with queue-backed compare execution in Sail/Docker
Project Type: web application in a monorepo (apps/platform plus apps/website)
Performance Goals: Preserve current compare start latency and compare job throughput, add no new remote calls or DB writes, and keep operator-facing compare and monitoring surfaces behaviorally unchanged
Constraints: No behavior change, no new abstraction or persistence, no operator-facing surface changes, no OperationRun lifecycle changes, and keep the PR mechanically small and reviewable
Scale/Scope: One queued compare job, one compare start service boundary, one active strategy registry, one active Intune strategy, existing finding and run writers, and a small focused regression slice
Constitution Check
GATE: Passed before Phase 0 research. Re-checked after Phase 1 design and still passing because the feature removes code without altering scope, auth, persistence, or UI contracts.
| Principle | Pre-Research | Post-Design | Notes |
|---|---|---|---|
| Inventory-first / snapshots-second | PASS | PASS | Compare still reads existing workspace baseline snapshots and inventory-backed current state; no new compare truth is introduced. |
| Read/write separation | PASS | PASS | Existing compare runs still write only current run, finding, and audit truth; the cleanup adds no new write path. |
| Graph contract path | PASS | PASS | No new Microsoft Graph path or contract is introduced. |
| Deterministic capabilities | PASS | PASS | Existing strategy selection and capability behavior remain unchanged because the registry and strategy classes stay intact. |
| Workspace + tenant isolation | PASS | PASS | No workspace, tenant, or route-scope behavior changes are planned. |
| RBAC-UX authorization semantics | PASS | PASS | No authorization rules, capability checks, or cross-plane behavior are changed. |
| Run observability / Ops-UX | PASS | PASS | Existing baseline_compare run creation, summary counts, and completion semantics remain authoritative and unchanged. |
| Data minimization | PASS | PASS | No new persisted diagnostics or helper truth is added; dead internal code is removed instead. |
| Proportionality / anti-bloat | PASS | PASS | The feature deletes an obsolete path and introduces no new structure. |
| No premature abstraction | PASS | PASS | No new factory, resolver, registry, strategy, or support layer is introduced. |
| Persisted truth / behavioral state | PASS | PASS | No new table, stored artifact, status family, or reason family is added. |
| UI semantics / few layers | PASS | PASS | No new presentation layer or surface behavior is introduced. |
| Filament v5 / Livewire v4 compliance | PASS | PASS | No Filament or Livewire API changes are part of this cleanup. |
| Provider registration location | PASS | PASS | No panel or provider change is required; Laravel 11+ provider registration remains in bootstrap/providers.php. |
| Global search hard rule | PASS | PASS | No searchable resource or search behavior is touched. |
| Destructive action safety | PASS | PASS | No destructive action is added or changed. |
| Asset strategy | PASS | PASS | No new assets are introduced and existing filament:assets deployment behavior remains unchanged. |
Filament-Specific Compliance Notes
- Livewire v4.0+ compliance: Unchanged. The feature touches no Filament surface or Livewire component and does not introduce legacy APIs.
- Provider registration location: Unchanged. If any panel/provider review is needed later, Laravel 11+ still requires
bootstrap/providers.php. - Global search: No globally searchable resource is added or changed.
- Destructive actions: No destructive action is introduced; existing confirmation and authorization rules remain untouched.
- Asset strategy: No new panel or shared assets are required. Deployment handling of
cd apps/platform && php artisan filament:assetsremains unchanged. - Testing plan: Focus on the required compare cleanup pack only: compare execution and findings regressions, gap and reason-code coverage,
Spec116OneEngineGuardTest,Spec118NoLegacyBaselineDriftGuardTest, existingOperationRunand summary-count guards, and the enqueue-path matrix action regression. No new page, widget, relation manager, or action surface coverage is required.
Phase 0 Research
Research outcomes are captured in /Users/ahmeddarrazi/Documents/projects/TenantAtlas/specs/205-compare-job-cleanup/research.md.
Key decisions:
- Treat the current
CompareStrategyRegistry->IntuneCompareStrategyexecution path as the only supported compare engine. - Delete the dead
computeDrift()cluster rather than retaining it as deprecated or archived code. - Preserve
CompareSubjectResult, finding upsert, summary aggregation, gap handling, and run completion semantics exactly as they currently operate through the live strategy path. - Use one focused compare pack covering execution fidelity, finding lifecycle, gap and reason outcomes,
OperationRunlifecycle guards, summary-count guards, and the no-legacy orchestration guard as the minimum reliable regression slice. - Keep the contract artifact logical and internal, documenting invariants of the unchanged execution boundary instead of inventing a new external API.
Phase 1 Design
Design artifacts are created under /Users/ahmeddarrazi/Documents/projects/TenantAtlas/specs/205-compare-job-cleanup/:
research.md: cleanup decisions, rationale, and rejected alternativesdata-model.md: existing persisted truth and internal compare contracts preserved by the cleanupcontracts/compare-job-legacy-drift-cleanup.logical.openapi.yaml: logical internal contract for the unchanged compare start and execution boundaries plus the no-legacy guard invariantquickstart.md: implementation and verification order for the cleanup
Design decisions:
CompareBaselineToTenantJobremains the compare execution entry point, but only the live orchestration methods stay after cleanup.CompareStrategyRegistry,IntuneCompareStrategy,CompareStrategySelection,CompareOrchestrationContext,CompareSubjectResult, andCompareFindingCandidateremain reused unchanged.- Existing persisted truth in baseline snapshots, inventory, findings, and
operation_runsremains authoritative; no migration or compatibility layer is added. - Guard coverage remains the explicit enforcement point preventing legacy drift computation from re-entering the orchestration file.
- Existing
OperationRunlifecycle and summary-count guards remain part of the required verification surface because the cleanup still edits the compare executor. - No route, UI, RBAC, or
OperationRundesign change is planned.
Project Structure
Documentation (this feature)
specs/205-compare-job-cleanup/
├── plan.md
├── research.md
├── data-model.md
├── quickstart.md
├── spec.md
├── contracts/
│ └── compare-job-legacy-drift-cleanup.logical.openapi.yaml
└── checklists/
└── requirements.md
Source Code (repository root)
apps/platform/
├── app/
│ ├── Jobs/
│ │ └── CompareBaselineToTenantJob.php
│ ├── Services/
│ │ └── Baselines/
│ │ ├── BaselineCompareService.php
│ │ ├── CurrentStateHashResolver.php
│ │ └── Evidence/
│ └── Support/
│ └── Baselines/
│ └── Compare/
│ ├── CompareStrategyRegistry.php
│ ├── CompareStrategySelection.php
│ ├── CompareSubjectResult.php
│ ├── CompareFindingCandidate.php
│ └── IntuneCompareStrategy.php
└── tests/
├── Feature/
│ ├── BaselineDriftEngine/
│ │ └── FindingFidelityTest.php
│ ├── Baselines/
│ │ ├── BaselineCompareFindingsTest.php
│ │ ├── BaselineCompareGapClassificationTest.php
│ │ ├── BaselineCompareWhyNoFindingsReasonCodeTest.php
│ │ └── BaselineCompareMatrixCompareAllActionTest.php
│ └── Guards/
│ ├── Spec116OneEngineGuardTest.php
│ ├── Spec118NoLegacyBaselineDriftGuardTest.php
│ └── OperationLifecycleOpsUxGuardTest.php
│ ├── Operations/
│ │ └── BaselineOperationRunGuardTest.php
│ └── OpsUx/
│ ├── OperationSummaryKeysSpecTest.php
│ └── SummaryCountsWhitelistTest.php
Structure Decision: Keep the cleanup inside the existing compare orchestration file and current compare regression surfaces. No new namespace, support layer, or package structure is introduced.
Complexity Tracking
No constitution exception or complexity justification is required. Spec 205 removes an obsolete implementation branch and introduces no new persistence, abstraction, state family, or semantic framework.
Proportionality Review
Not triggered. This feature introduces no new enum or status family, DTO or presenter layer, persisted artifact, interface or registry, or cross-domain taxonomy.
Implementation Strategy
Phase A - Confirm dead call graph
- Confirm that
handle()no longer reachescomputeDrift()or its candidate helper cluster. - Confirm that the live path still runs through strategy selection, strategy resolution,
strategy->compare(...), normalization, finding upsert, and run completion. - Record the final helper delete list before editing to avoid removing shared orchestration methods.
Phase B - Delete the legacy drift cluster
- Remove
computeDrift()and every helper method used exclusively by that legacy path. - Remove only the imports, comments, and internal descriptions that become dead because of the delete.
- Keep shared orchestration helpers such as evidence resolution, result normalization, summary aggregation, gap merging, and finding lifecycle methods untouched.
Phase C - Preserve the live orchestration contract
- Leave
BaselineCompareService,CompareStrategyRegistry, andIntuneCompareStrategybehavior unchanged unless a direct compile or test failure requires a minimal follow-up. - Preserve the existing
baseline_comparerun context shape, summary count rules, gap handling, reason translation, and finding lifecycle semantics. - Avoid any naming sweep, contract redesign, or opportunistic cleanup outside the dead cluster.
Phase D - Guard and regression verification
- Keep or tighten the existing no-legacy guard so the removed path cannot silently re-enter the orchestration file.
- Run focused compare execution, gap and reason-code regression, and
OperationRunlifecycle and summary-count guard tests to prove the delete is mechanically safe. - Format the touched PHP files with Pint after the cleanup is implemented.
Risk Assessment
| Risk | Impact | Likelihood | Mitigation |
|---|---|---|---|
| A supposedly dead helper is still used by the live orchestration path | High | Medium | Confirm call sites before deletion and keep the initial regression pack focused on execution, findings, gap and reason outcomes, and run-lifecycle guards. |
| The cleanup grows into a broader refactor while the job file is open | Medium | Medium | Constrain edits to dead methods, direct import fallout, and guard or test changes required by the delete. |
| Existing guard tests are too weak or too token-specific to prevent reintroduction | Medium | Medium | Reuse Spec118NoLegacyBaselineDriftGuardTest and extend it with the removed method names only if the current assertions do not cover the dead-path cluster clearly enough. |
| A regression test depends on deleted internal structure rather than behavior | Medium | Low | Update such tests to assert live compare outcomes and orchestration invariants rather than private helper presence. |
Test Strategy
- Run
tests/Feature/BaselineDriftEngine/FindingFidelityTest.phpas the primary execution and evidence-fidelity regression slice. - Run
tests/Feature/Baselines/BaselineCompareFindingsTest.phpto protect finding generation, recurrence, summary counts, and run completion outcomes. - Run
tests/Feature/Baselines/BaselineCompareGapClassificationTest.phpandtests/Feature/Baselines/BaselineCompareWhyNoFindingsReasonCodeTest.phpto protect gap handling, warning outcomes, and reason translation behavior. - Run
tests/Feature/Guards/Spec116OneEngineGuardTest.phpto keep the one-engine orchestration invariant explicit while the dead fallback cluster is removed. - Run
tests/Feature/Guards/OperationLifecycleOpsUxGuardTest.php,tests/Feature/Operations/BaselineOperationRunGuardTest.php,tests/Feature/OpsUx/OperationSummaryKeysSpecTest.php, andtests/Feature/OpsUx/SummaryCountsWhitelistTest.phpto keepOperationRunlifecycle and summary-count guarantees intact. - Run
tests/Feature/Guards/Spec118NoLegacyBaselineDriftGuardTest.phpto lock the orchestration boundary against legacy drift helper re-entry. - Run
tests/Feature/Baselines/BaselineCompareMatrixCompareAllActionTest.phpas the required enqueue-path regression slice for the focused cleanup pack. - Run
cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agentafter the code change.