## 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
194 lines
16 KiB
Markdown
194 lines
16 KiB
Markdown
# Tasks: Compare Job Legacy Drift Path Cleanup
|
|
|
|
**Input**: Design documents from `/specs/205-compare-job-cleanup/`
|
|
**Prerequisites**: `plan.md`, `spec.md`, `research.md`, `data-model.md`, `contracts/compare-job-legacy-drift-cleanup.logical.openapi.yaml`, `quickstart.md`
|
|
|
|
**Tests**: Required. This cleanup changes runtime compare orchestration code in `CompareBaselineToTenantJob` and must keep the current strategy-driven compare path green through focused Pest regression and guard coverage.
|
|
**Operations**: Existing `baseline_compare` `OperationRun` behavior remains unchanged. No new run type, feedback surface, or monitoring path is introduced.
|
|
**RBAC**: No authorization change is in scope. Existing compare and monitoring permissions remain authoritative, and tasks must avoid introducing RBAC drift while touching the orchestration file.
|
|
**Operator Surfaces**: No operator-facing surface change is in scope. Existing tenant compare and monitoring routes remain verification anchors only.
|
|
**Filament UI Action Surfaces**: No Filament resource, page, relation manager, or action-hierarchy change is planned.
|
|
**Proportionality**: This spec removes dead code only and must not introduce new abstractions, persistence, or semantic layers.
|
|
|
|
**Organization**: Tasks are grouped by user story so the cleanup can be implemented and verified in narrow, reviewable increments. Recommended delivery order is `US1 -> US2 -> US3`, with `US1 + US2` forming the practical merge-ready slice.
|
|
|
|
## Phase 1: Setup (Shared Baseline)
|
|
|
|
**Purpose**: Capture the full required pre-cleanup regression baseline and inspect the active compare boundary before editing the orchestration file.
|
|
|
|
- [X] T001 [P] Capture the required pre-cleanup regression baseline by running `apps/platform/tests/Feature/BaselineDriftEngine/FindingFidelityTest.php`, `apps/platform/tests/Feature/Baselines/BaselineCompareFindingsTest.php`, `apps/platform/tests/Feature/Baselines/BaselineCompareGapClassificationTest.php`, `apps/platform/tests/Feature/Baselines/BaselineCompareWhyNoFindingsReasonCodeTest.php`, `apps/platform/tests/Feature/Guards/Spec116OneEngineGuardTest.php`, `apps/platform/tests/Feature/Guards/Spec118NoLegacyBaselineDriftGuardTest.php`, `apps/platform/tests/Feature/Guards/OperationLifecycleOpsUxGuardTest.php`, `apps/platform/tests/Feature/Operations/BaselineOperationRunGuardTest.php`, `apps/platform/tests/Feature/OpsUx/OperationSummaryKeysSpecTest.php`, `apps/platform/tests/Feature/OpsUx/SummaryCountsWhitelistTest.php`, and `apps/platform/tests/Feature/Baselines/BaselineCompareMatrixCompareAllActionTest.php`
|
|
- [X] T002 [P] Inspect the live compare dispatch and candidate legacy helper cluster in `apps/platform/app/Jobs/CompareBaselineToTenantJob.php` and `apps/platform/app/Services/Baselines/BaselineCompareService.php`
|
|
|
|
**Checkpoint**: The team has a known-good full focused baseline and a confirmed starting map of the live compare path.
|
|
|
|
---
|
|
|
|
## Phase 2: Foundational (Blocking Call-Graph Confirmation)
|
|
|
|
**Purpose**: Confirm the dead-vs-live method boundary so the cleanup deletes only unreachable logic.
|
|
|
|
**CRITICAL**: No user story work should begin until this phase is complete.
|
|
|
|
- [X] T003 [P] Map exclusive callers for `computeDrift()` and its adjacent helper cluster in `apps/platform/app/Jobs/CompareBaselineToTenantJob.php`
|
|
- [X] T004 [P] Review `apps/platform/app/Support/Baselines/Compare/CompareStrategyRegistry.php` and `apps/platform/app/Support/Baselines/Compare/IntuneCompareStrategy.php` to confirm the live strategy contract needs no structural change for this cleanup
|
|
|
|
**Checkpoint**: The delete list is confirmed and the live strategy-owned path is explicitly out of scope for redesign.
|
|
|
|
---
|
|
|
|
## Phase 3: User Story 1 - Read the live compare architecture without dead-path noise (Priority: P1) MVP
|
|
|
|
**Goal**: Remove the retained monolithic drift-compute path so the compare job shows one real execution engine instead of a parallel historical implementation.
|
|
|
|
**Independent Test**: Inspect `apps/platform/app/Jobs/CompareBaselineToTenantJob.php` after cleanup and confirm that the live compare path still flows through strategy selection and `strategy->compare(...)`, while the legacy helper names are absent and the guard suite passes.
|
|
|
|
### Tests for User Story 1
|
|
|
|
> **NOTE**: Update these tests first and confirm they fail before implementation.
|
|
|
|
- [X] T005 [P] [US1] Extend legacy helper absence assertions in `apps/platform/tests/Feature/Guards/Spec118NoLegacyBaselineDriftGuardTest.php`
|
|
- [X] T006 [P] [US1] Reconfirm one-engine orchestration guard coverage in `apps/platform/tests/Feature/Guards/Spec116OneEngineGuardTest.php`
|
|
|
|
### Implementation for User Story 1
|
|
|
|
- [X] T007 [US1] Remove `computeDrift()` and its exclusive helper cluster from `apps/platform/app/Jobs/CompareBaselineToTenantJob.php`
|
|
- [X] T008 [US1] Remove dead imports and stale fallback comments or docblocks left by the deleted cluster in `apps/platform/app/Jobs/CompareBaselineToTenantJob.php`
|
|
- [X] T009 [US1] Re-run the guard coverage in `apps/platform/tests/Feature/Guards/Spec116OneEngineGuardTest.php` and `apps/platform/tests/Feature/Guards/Spec118NoLegacyBaselineDriftGuardTest.php` against `apps/platform/app/Jobs/CompareBaselineToTenantJob.php`
|
|
|
|
**Checkpoint**: The compare job is structurally honest again and the guard suite blocks reintroduction of the deleted legacy path.
|
|
|
|
---
|
|
|
|
## Phase 4: User Story 2 - Preserve current compare behavior while removing dead code (Priority: P1)
|
|
|
|
**Goal**: Prove that the cleanup leaves strategy selection, compare execution, finding lifecycle behavior, summary computation, gap handling, warning handling, reason translation, and run completion unchanged.
|
|
|
|
**Independent Test**: Run the required focused regression slice and confirm that `FindingFidelityTest`, `BaselineCompareFindingsTest`, `BaselineCompareGapClassificationTest`, `BaselineCompareWhyNoFindingsReasonCodeTest`, `Spec116OneEngineGuardTest`, `Spec118NoLegacyBaselineDriftGuardTest`, `OperationLifecycleOpsUxGuardTest`, `BaselineOperationRunGuardTest`, `OperationSummaryKeysSpecTest`, `SummaryCountsWhitelistTest`, and the matrix enqueue-path check all remain green after the delete.
|
|
|
|
### Tests for User Story 2
|
|
|
|
> **NOTE**: Update these tests first and confirm they fail before implementation.
|
|
|
|
- [X] T010 [P] [US2] Tighten strategy-driven execution assertions in `apps/platform/tests/Feature/BaselineDriftEngine/FindingFidelityTest.php`
|
|
- [X] T011 [P] [US2] Tighten finding lifecycle, recurrence, and summary outcome assertions in `apps/platform/tests/Feature/Baselines/BaselineCompareFindingsTest.php`
|
|
- [X] T012 [P] [US2] Tighten gap classification, warning-outcome, and reason-code assertions in `apps/platform/tests/Feature/Baselines/BaselineCompareGapClassificationTest.php` and `apps/platform/tests/Feature/Baselines/BaselineCompareWhyNoFindingsReasonCodeTest.php`
|
|
- [X] T013 [P] [US2] Reconfirm `OperationRun` lifecycle and summary-count guard coverage in `apps/platform/tests/Feature/Guards/OperationLifecycleOpsUxGuardTest.php`, `apps/platform/tests/Feature/Operations/BaselineOperationRunGuardTest.php`, `apps/platform/tests/Feature/OpsUx/OperationSummaryKeysSpecTest.php`, and `apps/platform/tests/Feature/OpsUx/SummaryCountsWhitelistTest.php`
|
|
|
|
### Implementation for User Story 2
|
|
|
|
- [X] T014 [US2] Keep live compare behavior unchanged while reconciling any cleanup fallout in `apps/platform/app/Jobs/CompareBaselineToTenantJob.php`; do not modify `apps/platform/app/Support/Baselines/Compare/CompareStrategyRegistry.php` or `apps/platform/app/Support/Baselines/Compare/IntuneCompareStrategy.php` unless a blocker proves the smallest direct fix is required
|
|
- [X] T015 [US2] Run the required focused regression slice in `apps/platform/tests/Feature/BaselineDriftEngine/FindingFidelityTest.php`, `apps/platform/tests/Feature/Baselines/BaselineCompareFindingsTest.php`, `apps/platform/tests/Feature/Baselines/BaselineCompareGapClassificationTest.php`, `apps/platform/tests/Feature/Baselines/BaselineCompareWhyNoFindingsReasonCodeTest.php`, `apps/platform/tests/Feature/Guards/Spec116OneEngineGuardTest.php`, `apps/platform/tests/Feature/Guards/Spec118NoLegacyBaselineDriftGuardTest.php`, `apps/platform/tests/Feature/Guards/OperationLifecycleOpsUxGuardTest.php`, `apps/platform/tests/Feature/Operations/BaselineOperationRunGuardTest.php`, `apps/platform/tests/Feature/OpsUx/OperationSummaryKeysSpecTest.php`, `apps/platform/tests/Feature/OpsUx/SummaryCountsWhitelistTest.php`, and `apps/platform/tests/Feature/Baselines/BaselineCompareMatrixCompareAllActionTest.php`
|
|
|
|
**Checkpoint**: The cleanup is behaviorally safe and the current compare lifecycle still works through the strategy-owned path.
|
|
|
|
---
|
|
|
|
## Phase 5: User Story 3 - Keep the review diff mechanically narrow (Priority: P2)
|
|
|
|
**Goal**: Keep the cleanup PR reviewable by limiting the touched area to dead-path deletion, direct fallout cleanup, and minimal regression updates.
|
|
|
|
**Independent Test**: Inspect the final changed-file set and confirm that it is limited to `apps/platform/app/Jobs/CompareBaselineToTenantJob.php`, the focused compare guard and regression files, and only direct blocker-driven follow-up in `apps/platform/app/Services/Baselines/BaselineCompareService.php` or `apps/platform/app/Support/Baselines/Compare/IntuneCompareStrategy.php` if justified.
|
|
|
|
### Implementation for User Story 3
|
|
|
|
- [X] T016 [P] [US3] Audit the touched-file set and strip opportunistic edits outside `apps/platform/app/Jobs/CompareBaselineToTenantJob.php`, `apps/platform/tests/Feature/Guards/Spec116OneEngineGuardTest.php`, `apps/platform/tests/Feature/Guards/Spec118NoLegacyBaselineDriftGuardTest.php`, `apps/platform/tests/Feature/BaselineDriftEngine/FindingFidelityTest.php`, `apps/platform/tests/Feature/Baselines/BaselineCompareFindingsTest.php`, `apps/platform/tests/Feature/Baselines/BaselineCompareGapClassificationTest.php`, `apps/platform/tests/Feature/Baselines/BaselineCompareWhyNoFindingsReasonCodeTest.php`, `apps/platform/tests/Feature/Guards/OperationLifecycleOpsUxGuardTest.php`, `apps/platform/tests/Feature/Operations/BaselineOperationRunGuardTest.php`, `apps/platform/tests/Feature/OpsUx/OperationSummaryKeysSpecTest.php`, `apps/platform/tests/Feature/OpsUx/SummaryCountsWhitelistTest.php`, and `apps/platform/tests/Feature/Baselines/BaselineCompareMatrixCompareAllActionTest.php`
|
|
- [X] T017 [P] [US3] Review `apps/platform/app/Services/Baselines/BaselineCompareService.php` and `apps/platform/app/Support/Baselines/Compare/IntuneCompareStrategy.php` to confirm they remain unchanged or carry only the smallest blocker-driven follow-up required by the cleanup
|
|
- [X] T018 [US3] Verify the final diff stays limited to `apps/platform/app/Jobs/CompareBaselineToTenantJob.php`, the focused compare guard and regression files above including `apps/platform/tests/Feature/Baselines/BaselineCompareMatrixCompareAllActionTest.php`, and only direct blocker-driven follow-up in `apps/platform/app/Services/Baselines/BaselineCompareService.php` or `apps/platform/app/Support/Baselines/Compare/IntuneCompareStrategy.php`
|
|
|
|
**Checkpoint**: The PR stays small, reviewable, and aligned with Spec 205 rather than drifting into a broader compare refactor.
|
|
|
|
---
|
|
|
|
## Phase 6: Polish & Cross-Cutting Concerns
|
|
|
|
**Purpose**: Apply formatting and rerun the final focused Sail pack before handing the cleanup over for review.
|
|
|
|
- [X] T019 Run `cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent` for touched PHP files centered on `apps/platform/app/Jobs/CompareBaselineToTenantJob.php`
|
|
- [X] T020 Run the final focused Sail pack from `specs/205-compare-job-cleanup/quickstart.md` covering `apps/platform/tests/Feature/BaselineDriftEngine/FindingFidelityTest.php`, `apps/platform/tests/Feature/Baselines/BaselineCompareFindingsTest.php`, `apps/platform/tests/Feature/Baselines/BaselineCompareGapClassificationTest.php`, `apps/platform/tests/Feature/Baselines/BaselineCompareWhyNoFindingsReasonCodeTest.php`, `apps/platform/tests/Feature/Guards/OperationLifecycleOpsUxGuardTest.php`, `apps/platform/tests/Feature/Operations/BaselineOperationRunGuardTest.php`, `apps/platform/tests/Feature/OpsUx/OperationSummaryKeysSpecTest.php`, `apps/platform/tests/Feature/OpsUx/SummaryCountsWhitelistTest.php`, `apps/platform/tests/Feature/Guards/Spec116OneEngineGuardTest.php`, `apps/platform/tests/Feature/Guards/Spec118NoLegacyBaselineDriftGuardTest.php`, and `apps/platform/tests/Feature/Baselines/BaselineCompareMatrixCompareAllActionTest.php`
|
|
|
|
---
|
|
|
|
## Dependencies & Execution Order
|
|
|
|
### Phase Dependencies
|
|
|
|
- **Setup (Phase 1)**: No dependencies; can start immediately.
|
|
- **Foundational (Phase 2)**: Depends on Setup completion; blocks all user story work.
|
|
- **User Story 1 (Phase 3)**: Depends on Foundational completion.
|
|
- **User Story 2 (Phase 4)**: Depends on User Story 1 because behavior preservation is verified after the delete lands.
|
|
- **User Story 3 (Phase 5)**: Depends on User Story 1 and User Story 2 because scope review is only meaningful once the delete, gap and reason coverage, and run-guard updates exist.
|
|
- **Polish (Phase 6)**: Depends on all user stories being complete.
|
|
|
|
### User Story Dependencies
|
|
|
|
- **US1**: No dependency beyond Foundational.
|
|
- **US2**: Depends on US1 because the focused regression slice validates the actual cleanup result.
|
|
- **US3**: Depends on US1 and US2 because it is the final scope-control pass over the implemented cleanup.
|
|
|
|
### Within Each User Story
|
|
|
|
- Update or tighten the story's tests first and confirm they fail before implementation.
|
|
- Keep compare start orchestration in `apps/platform/app/Services/Baselines/BaselineCompareService.php` and live strategy behavior in `apps/platform/app/Support/Baselines/Compare/IntuneCompareStrategy.php` out of scope unless a blocker demands the smallest possible fix.
|
|
- Finish each story's focused verification before moving to the next story.
|
|
|
|
### Parallel Opportunities
|
|
|
|
- `T001` and `T002` can run in parallel.
|
|
- `T003` and `T004` can run in parallel.
|
|
- Within US1, `T005` and `T006` can run in parallel.
|
|
- Within US2, `T010`, `T011`, `T012`, and `T013` can run in parallel.
|
|
- Within US3, `T016` and `T017` can run in parallel.
|
|
|
|
---
|
|
|
|
## Parallel Example: User Story 1
|
|
|
|
```bash
|
|
# Parallel guard updates for US1
|
|
T005 Extend legacy helper absence assertions in Spec118NoLegacyBaselineDriftGuardTest.php
|
|
T006 Reconfirm one-engine orchestration guard coverage in Spec116OneEngineGuardTest.php
|
|
```
|
|
|
|
## Parallel Example: User Story 2
|
|
|
|
```bash
|
|
# Parallel regression tightening for US2
|
|
T010 Tighten strategy-driven execution assertions in FindingFidelityTest.php
|
|
T011 Tighten finding lifecycle and summary outcome assertions in BaselineCompareFindingsTest.php
|
|
T012 Tighten gap classification and reason-code assertions in BaselineCompareGapClassificationTest.php and BaselineCompareWhyNoFindingsReasonCodeTest.php
|
|
T013 Reconfirm OperationRun lifecycle and summary-count guard coverage in OperationLifecycleOpsUxGuardTest.php, BaselineOperationRunGuardTest.php, OperationSummaryKeysSpecTest.php, and SummaryCountsWhitelistTest.php
|
|
```
|
|
|
|
## Parallel Example: User Story 3
|
|
|
|
```bash
|
|
# Parallel scope review for US3
|
|
T016 Audit the touched-file set and strip opportunistic edits outside the focused cleanup files
|
|
T017 Review BaselineCompareService.php and IntuneCompareStrategy.php for blocker-only follow-up changes
|
|
```
|
|
|
|
---
|
|
|
|
## Implementation Strategy
|
|
|
|
### MVP First
|
|
|
|
1. Complete Setup and Foundational work.
|
|
2. Deliver US1 to remove the dead path and restore a single truthful compare engine in the orchestration file.
|
|
3. Immediately follow with US2 so the cleanup is merge-safe, not just structurally cleaner.
|
|
|
|
### Incremental Delivery
|
|
|
|
1. Finish US1 and confirm the guard suite blocks the deleted helper cluster.
|
|
2. Finish US2 and prove the live strategy-driven compare behavior remains unchanged.
|
|
3. Finish US3 to keep the cleanup PR mechanically narrow.
|
|
4. Finish with formatting and the final focused Sail pack from Phase 6.
|
|
|
|
### Parallel Team Strategy
|
|
|
|
1. One contributor handles Setup and Foundational call-graph confirmation.
|
|
2. After Foundation is green:
|
|
T005 and T006 can be prepared in parallel for US1.
|
|
T010, T011, T012, and T013 can be prepared in parallel for US2.
|
|
T016 and T017 can be prepared in parallel for US3 once the cleanup diff exists.
|
|
3. Merge back for the final diff review, formatting, and focused Sail verification. |