ahmido/TenantAtlas
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects 1 Releases Wiki Activity
feat/196-hard-filament-nativity-cleanup
TenantAtlas/specs/205-compare-job-cleanup/research.md
ahmido bb72a54e84 Refactor: remove compare job legacy drift path (#235) 
## 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
2026-04-14 21:54:37 +00:00

4.6 KiB
Raw Permalink Blame History

Research: Compare Job Legacy Drift Path Cleanup

Decision 1: Treat the strategy-driven compare path as the only authoritative execution engine

  • Decision: Use the existing CompareStrategyRegistry -> IntuneCompareStrategy path as the sole supported compare execution boundary.
  • Rationale: Current code inspection shows CompareBaselineToTenantJob::handle() selecting a strategy, resolving it, and calling strategy->compare(...) before normalizing subject results and writing findings. No productive call path from handle() reaches the retained monolithic computeDrift() block.
  • Alternatives considered:
    • Keep the legacy block as a documented fallback. Rejected because it leaves the file structurally dishonest and suggests a second engine still exists.
    • Add a feature flag between strategy and legacy execution. Rejected because there is no legitimate second execution mode left to preserve.

Decision 2: Delete the dead drift cluster instead of archiving or deprecating it

  • Decision: Remove computeDrift() and its exclusive helper cluster directly from CompareBaselineToTenantJob.
  • Rationale: The retained cluster duplicates pre-strategy compare logic that has already been extracted into the active strategy implementation. Keeping it in place continues to mislead reviewers and inflates the orchestration file without operational value.
  • Alternatives considered:
    • Move the dead methods to a trait or archive class. Rejected because it preserves confusion and ownership cost without any runtime benefit.
    • Leave the methods in place with a deprecation comment. Rejected because dead code still obscures the real call graph even when labeled.

Decision 3: Preserve existing compare contracts, findings, and run semantics unchanged

  • Decision: Keep CompareStrategyRegistry, IntuneCompareStrategy, CompareStrategySelection, CompareSubjectResult, CompareFindingCandidate, existing finding writers, and existing run context semantics unchanged.
  • Rationale: Spec 205 is a closure cleanup, not a second strategy extraction spec. The safest path is deletion of dead code while leaving the live contracts and persisted truths untouched.
  • Alternatives considered:
    • Fold in additional compare refactors while editing the job. Rejected because that turns a narrow cleanup into a mixed review.
    • Rename or reframe current compare contracts for symmetry. Rejected because it is unrelated to dead-path removal.

Decision 4: Use a focused compare plus run-guard pack as the minimum regression slice

  • Decision: Validate the cleanup with FindingFidelityTest, BaselineCompareFindingsTest, BaselineCompareGapClassificationTest, BaselineCompareWhyNoFindingsReasonCodeTest, Spec116OneEngineGuardTest, OperationLifecycleOpsUxGuardTest, BaselineOperationRunGuardTest, OperationSummaryKeysSpecTest, SummaryCountsWhitelistTest, Spec118NoLegacyBaselineDriftGuardTest, and BaselineCompareMatrixCompareAllActionTest as the required focused regression pack.
  • Rationale: FindingFidelityTest exercises the compare execution path and evidence selection behavior, BaselineCompareFindingsTest protects finding lifecycle and summary outcomes, the gap and reason-code tests protect warning and reason semantics, Spec116OneEngineGuardTest keeps the one-engine orchestration invariant explicit, the OperationRun and summary-count guards protect lifecycle invariants, and the legacy guard keeps helper re-entry visible in CI. Together they provide high confidence for a mechanical delete without requiring a broad slow suite.
  • Alternatives considered:
    • Run the full baseline compare suite for every cleanup iteration. Rejected as optional rather than required for a small internal delete.
    • Skip targeted tests and rely only on formatting or static inspection. Rejected as insufficient confidence.

Decision 5: Keep the planning artifacts logical and invariant-focused

  • Decision: Document the cleanup through a logical internal contract and a no-new-entity data model rather than inventing cleanup-specific services, APIs, or persistence.
  • Rationale: The plan workflow still needs explicit design artifacts, but Spec 205 adds no new feature surface. The correct documentation shape is therefore an invariant record of the unchanged compare boundaries after dead-code deletion.
  • Alternatives considered:
    • Skip the contract artifact entirely because no new endpoint exists. Rejected because the planning workflow requires a contract deliverable.
    • Invent a cleanup-specific service or endpoint in the design docs. Rejected because it would introduce fake architecture not warranted by the spec.