Some checks failed
PR Fast Feedback / fast-feedback (pull_request) Failing after 1m11s
Added UiBloatRegressionGuardTest to enforce known UI bloat and customer/auditor safety regression patterns across configured runtime UI source paths as defined in Spec 375. Registered the test in Pest.php and added to TestLaneManifest.
259 lines
17 KiB
Markdown
259 lines
17 KiB
Markdown
# Implementation Plan: UI Bloat Regression Guard v1
|
|
|
|
**Branch**: `375-ui-bloat-regression-guard` | **Date**: 2026-06-13 | **Spec**: `specs/375-ui-bloat-regression-guard/spec.md`
|
|
**Input**: Feature specification from `specs/375-ui-bloat-regression-guard/spec.md`
|
|
|
|
## Summary
|
|
|
|
Build a narrow repository guard that scans TenantPilot UI source paths for known UI bloat and customer/auditor safety regression patterns from Specs 368 and 370-374. The implementation should prefer existing Pest guard conventions and surface-guard lane ownership, create spec-local rule/design/allowlist/scan artifacts, run an initial scan in `warn` mode, and avoid any runtime UI page refactor.
|
|
|
|
## Technical Context
|
|
|
|
**Language/Version**: PHP 8.4.15, Laravel 12.52, Filament 5.2.1, Livewire 4.1.4
|
|
**Primary Dependencies**: Pest 4.3, PHPUnit 12, existing Laravel/Filament app and test harness
|
|
**Storage**: N/A - no database or product persistence; spec-local Markdown reports only
|
|
**Testing**: Pest guard/source-scan tests preferred; optional Artisan command test only if a command is selected
|
|
**Validation Lanes**: heavy-governance / surface-guard by default; targeted command/test for local validation
|
|
**Target Platform**: local Laravel app and Gitea-compatible CI runners
|
|
**Project Type**: Laravel monolith under `apps/platform` plus repo-level scripts/docs
|
|
**Performance Goals**: deterministic local scan; no browser, DB, provider, queue, or network dependency
|
|
**Constraints**: no runtime UI refactor; no fast-feedback lane pollution unless scan cost is proven narrow; hard-fail only clear customer/auditor default-surface leakage in v1
|
|
**Scale/Scope**: configured scan over selected UI source directories, not full repository or generated assets
|
|
|
|
## UI / Surface Guardrail Plan
|
|
|
|
- **Guardrail scope**: workflow-only guardrail change.
|
|
- **Affected routes/pages/actions/states/navigation/panel/provider surfaces**: none.
|
|
- **No-impact class, if applicable**: tooling-only / test-only / documentation-artifact change.
|
|
- **Native vs custom classification summary**: N/A - no rendered UI.
|
|
- **Shared-family relevance**: status messaging, header actions, dashboard/stat cards, evidence/report viewers, customer/auditor defaults, diagnostic entrypoints.
|
|
- **State layers in scope**: none for runtime; source-scan surface classification only.
|
|
- **Audience modes in scope**: customer/read-only, operator-MSP, support-platform as scanner classifications.
|
|
- **Decision/diagnostic/raw hierarchy plan**: derive guard rules from Spec 370-374 hierarchy: decision/customer-safe content first, diagnostics second, support/raw third.
|
|
- **Raw/support gating plan**: guard reports raw/support terms in customer/auditor defaults as blocking or manual review depending on context/allowlist.
|
|
- **One-primary-action / duplicate-truth control**: scanner flags missing primary question/next action, header action overload, repeated status/lifecycle/timestamp/count noise, and zero-card spam.
|
|
- **Handling modes by drift class or surface**: clear customer/auditor leakage is hard-stop candidate; diagnostic/operator ambiguity is review-mandatory; allowlisted legacy debt is report-only.
|
|
- **Repository-signal treatment**: report/warn by default, future hard-stop candidate for additional rules after allowlist cleanup.
|
|
- **Special surface test profiles**: surface-guard; no browser smoke.
|
|
- **Required tests or manual smoke**: targeted source-scan fixture/sample coverage and initial scan report.
|
|
- **Exception path and spread control**: allowlist entries must be explicit, reasoned, reviewed, and scoped by rule/file/pattern.
|
|
- **Active feature PR close-out entry**: Guardrail.
|
|
- **UI/Productization coverage decision**: No UI surface impact.
|
|
- **Coverage artifacts to update**: none.
|
|
- **No-impact rationale**: no reachable UI is added, removed, renamed, or materially changed.
|
|
- **Navigation / Filament provider-panel handling**: no panel/provider changes.
|
|
- **Screenshot or page-report need**: no; guard-only spec.
|
|
|
|
## Shared Pattern & System Fit
|
|
|
|
- **Cross-cutting feature marker**: yes, because the guard reviews cross-surface UI interaction classes; no runtime shared interaction family changes.
|
|
- **Systems touched**: likely `apps/platform/tests/Feature/Guards`, `apps/platform/tests/Architecture`, `apps/platform/tests/Pest.php`, `apps/platform/tests/Support/TestLaneManifest.php` when heavy-governance ownership is selected, optional `apps/platform/app/Console/Commands`, optional repo scripts, and spec-local artifacts.
|
|
- **Shared abstractions reused**: existing source-scan guard patterns, Pest group/lane conventions, and `scripts/check-ui-productization-coverage` as a reference for report/fail behavior.
|
|
- **New abstraction introduced? why?**: at most one narrow scanner helper or guard test. It is justified only if inline test code would be too duplicated or unreadable.
|
|
- **Why the existing abstraction was sufficient or insufficient**: current UI coverage guard checks acknowledgement of UI impact, not source content bloat or customer-safety leakage.
|
|
- **Bounded deviation / spread control**: no product runtime abstraction; no framework under `app/Support` unless implementation proves test-local code is insufficient.
|
|
|
|
## OperationRun UX Impact
|
|
|
|
- **Touches OperationRun start/completion/link UX?**: no.
|
|
- **Central contract reused**: N/A.
|
|
- **Delegated UX behaviors**: N/A.
|
|
- **Surface-owned behavior kept local**: none.
|
|
- **Queued DB-notification policy**: N/A.
|
|
- **Terminal notification path**: N/A.
|
|
- **Exception path**: none.
|
|
|
|
## Provider Boundary & Portability Fit
|
|
|
|
- **Shared provider/platform boundary touched?**: no runtime provider seam.
|
|
- **Provider-owned seams**: N/A.
|
|
- **Platform-core seams**: N/A.
|
|
- **Neutral platform terms / contracts preserved**: guard language must describe provider/internal term leakage rather than making provider-specific labels platform truth.
|
|
- **Retained provider-specific semantics and why**: provider terms can remain legitimate in provider-owned diagnostic/operator surfaces and should be manual-review/allowlisted there.
|
|
- **Bounded extraction or follow-up path**: none.
|
|
|
|
## Constitution Check
|
|
|
|
*GATE: Must pass before implementation. Re-check after guard design is finalized.*
|
|
|
|
- Inventory-first: N/A - no inventory/snapshot behavior.
|
|
- Read/write separation: passes - no product write/change function; guard only reads source files and writes spec-local reports.
|
|
- Graph contract path: passes - no Graph calls.
|
|
- Deterministic capabilities: N/A - no capability resolver changes.
|
|
- RBAC-UX: passes - no authorization behavior changes; future implementation must not treat UI visibility as security.
|
|
- Workspace isolation: passes - no runtime workspace/tenant queries.
|
|
- Tenant isolation: passes - no tenant-owned data reads.
|
|
- Run observability: passes - no long-running remote/queued work and no OperationRun semantics.
|
|
- OperationRun start UX: N/A.
|
|
- Data minimization: guard reports must not include secrets, credentials, provider raw responses, or sensitive customer data.
|
|
- Test governance: surface-guard/heavy-governance classification is explicit; no hidden browser/DB fixtures.
|
|
- Proportionality: one narrow guard is justified by current UI safety/productization risk.
|
|
- No premature abstraction: prefer a single test/scanner path; avoid reusable framework unless needed for readability and tests.
|
|
- Persisted truth: no product persistence; spec-local reports only.
|
|
- Behavioral state: rule IDs are tooling diagnostics, not product state.
|
|
- UI semantics: guard consumes Spec 370-374 UI semantics and must not create a new UI interpretation framework.
|
|
- Shared pattern first: reuse existing guard/test/script conventions.
|
|
- Provider boundary: provider terms are scanner indicators only.
|
|
- V1 explicitness / few layers: direct text scanning and simple heuristics preferred over AST-heavy or browser-required tooling.
|
|
- Spec discipline / bloat check: proportionality review is complete.
|
|
- Filament-native UI: no rendered Filament UI changes; if implementation touches Filament source only by scanning it, no asset or provider updates are needed.
|
|
- UI/Productization coverage: no-impact decision is checked and rationalized.
|
|
|
|
## Test Governance Check
|
|
|
|
- **Test purpose / classification by changed surface**: Heavy-Governance / surface-guard for broad source scanning; unit-like sample cases may live inside the guard test.
|
|
- **Affected validation lanes**: heavy-governance or explicit targeted guard test/command; no browser lane.
|
|
- **Why this lane mix is the narrowest sufficient proof**: The feature protects cross-surface source patterns and does not render UI.
|
|
- **Narrowest proving command(s)**:
|
|
- `cd apps/platform && php vendor/bin/pest tests/Feature/Guards/UiBloatRegressionGuardTest.php` or exact selected guard file.
|
|
- If heavy-governance ownership is registered: targeted lane manifest/placement contract coverage proving the guard is discoverable by `test:heavy`, or a documented targeted-only validation if not registered.
|
|
- If an Artisan command is selected: `cd apps/platform && php artisan tenantpilot:check-ui-bloat --strictness=warn`.
|
|
- `git diff --check`.
|
|
- `cd apps/platform && php vendor/bin/pint --dirty` if PHP changed.
|
|
- **Fixture / helper / factory / seed / context cost risks**: none expected; use source fixture strings/files only.
|
|
- **Expensive defaults or shared helper growth introduced?**: no; any scanner helper must avoid DB/application boot beyond what Pest already needs.
|
|
- **Heavy-family additions, promotions, or visibility changes**: one new bounded surface-guard family or test.
|
|
- **Surface-class relief / special coverage rule**: no product page browser smoke.
|
|
- **Closing validation and reviewer handoff**: reviewer confirms no runtime UI refactor, no raw hard-fail expansion beyond customer/auditor defaults, no hidden fast-feedback lane cost, and report artifacts are complete.
|
|
- **Budget / baseline / trend follow-up**: record guard runtime and lane impact in validation report.
|
|
- **Review-stop questions**: lane fit, breadth, false positives, allowlist quality, hidden cost, and scope creep.
|
|
- **Escalation path**: document-in-feature; follow-up-spec only if CI hardening or browser integration becomes structural.
|
|
- **Active feature PR close-out entry**: Guardrail.
|
|
- **Why no dedicated follow-up spec is needed**: v1 guard setup is bounded; follow-up specs are limited to browser fixture coverage, scorecard integration, and CI strictness expansion.
|
|
|
|
## Project Structure
|
|
|
|
### Documentation (this feature)
|
|
|
|
```text
|
|
specs/375-ui-bloat-regression-guard/
|
|
|-- spec.md
|
|
|-- plan.md
|
|
|-- tasks.md
|
|
|-- checklists/
|
|
| `-- requirements.md
|
|
`-- artifacts/
|
|
|-- source-summary.md
|
|
|-- guard-rules.md
|
|
|-- scanner-design.md
|
|
|-- allowlist-policy.md
|
|
|-- initial-scan-report.md
|
|
|-- affected-files.md
|
|
|-- validation-report.md
|
|
`-- follow-up-recommendations.md
|
|
```
|
|
|
|
### Likely Source/Test Surfaces
|
|
|
|
```text
|
|
apps/platform/tests/Feature/Guards/
|
|
apps/platform/tests/Architecture/
|
|
apps/platform/tests/Pest.php
|
|
apps/platform/tests/Support/TestLaneManifest.php # if heavy-governance lane ownership is selected
|
|
apps/platform/app/Console/Commands/ # only if command is narrower than Pest-only guard
|
|
scripts/ # only if repo-level script is chosen
|
|
```
|
|
|
|
### Initial Scan Scope
|
|
|
|
```text
|
|
apps/platform/app/Filament
|
|
apps/platform/resources/views/filament
|
|
apps/platform/app/Support/EnvironmentDashboard
|
|
apps/platform/app/Support/Navigation
|
|
apps/platform/app/Support/OpsUx
|
|
apps/platform/app/Support/SupportDiagnostics
|
|
apps/platform/app/Support/Ui
|
|
apps/platform/app/Support/Workspaces
|
|
```
|
|
|
|
Candidate paths `apps/platform/resources/views/components` and `apps/platform/app/View` are not present in current repo truth and must be recorded as `not available` if still absent during implementation. Additional `apps/platform/app/Support` subpaths may be added only when `scanner-design.md` documents why they are UI-support paths; `apps/platform/app/Support` must not be scanned wholesale in v1.
|
|
|
|
### Exclusions / Separate Classification
|
|
|
|
```text
|
|
vendor
|
|
node_modules
|
|
storage
|
|
bootstrap/cache
|
|
public/build
|
|
database dumps
|
|
spec artifacts
|
|
screenshots
|
|
generated reports
|
|
tests unless explicitly used as scanner fixtures
|
|
translation dictionaries unless implementation proves rendered default UI copy is being checked
|
|
```
|
|
|
|
**Structure Decision**: Use a single repo-conform guard entrypoint. Prefer `apps/platform/tests/Feature/Guards/UiBloatRegressionGuardTest.php` because existing broad UI/source guard tests live under `tests/Feature/Guards`. If the guard is intended to run in `test:heavy`, it must be grouped in `apps/platform/tests/Pest.php` and registered as a `surface-guard` family/hotspot in `apps/platform/tests/Support/TestLaneManifest.php`; otherwise the source summary and validation report must document targeted-only ownership.
|
|
|
|
## Complexity Tracking
|
|
|
|
| Violation | Why Needed | Simpler Alternative Rejected Because |
|
|
|-----------|------------|-------------------------------------|
|
|
| New source scanner/guard | Existing UI coverage guard does not detect specific bloat/customer-safety copy patterns | Manual checklist alone does not reliably catch cross-surface drift |
|
|
| Rule IDs and allowlist policy | Findings need stable review language and controlled exceptions | Unstructured grep output would be noisy and unactionable |
|
|
|
|
## Proportionality Review
|
|
|
|
- **Current operator problem**: Future UI work can reintroduce unsafe customer/auditor copy and noisy, unprioritized surfaces without early review feedback.
|
|
- **Existing structure is insufficient because**: `scripts/check-ui-productization-coverage` proves coverage acknowledgement, not the content quality of changed source.
|
|
- **Narrowest correct implementation**: One source scanner/test/command, ten bounded rule groups, warn default, hard-fail only clear customer/auditor leakage, and spec-local artifacts.
|
|
- **Ownership cost created**: Rule maintenance, initial scan debt triage, allowlist review, and one guard lane/test.
|
|
- **Alternative intentionally rejected**: Full browser visual regression and broad screenshot diffs are too expensive and unnecessary for v1; page refactors are explicitly not this spec.
|
|
- **Release truth**: Current-release truth protecting completed UI productization.
|
|
|
|
## Implementation Phases
|
|
|
|
### Phase 1 - Repo Truth And Source Inputs
|
|
|
|
Verify existing guard/test/script conventions, existing console command patterns, Spec 368 inputs, and Spec 370-374 artifacts. Record available and missing inputs in `artifacts/source-summary.md`.
|
|
|
|
### Phase 2 - Guard Rules And Design Artifacts
|
|
|
|
Create `guard-rules.md`, `scanner-design.md`, and `allowlist-policy.md` before runtime/tooling changes so review can stop scope growth early.
|
|
|
|
### Phase 3 - Tests First / Fixture Cases
|
|
|
|
Add targeted sample cases proving customer/auditor hard failures, manual-review heuristics, allowlist shape, and strictness behavior.
|
|
|
|
### Phase 4 - Scanner / Entrypoint Implementation
|
|
|
|
Implement one repo-conform entrypoint. Prefer Pest guard unless final discovery proves Artisan command or script is narrower.
|
|
|
|
### Phase 5 - Initial Scan And Report
|
|
|
|
Run the guard in `warn` mode, generate `initial-scan-report.md`, and classify findings without broad UI fixes.
|
|
|
|
### Phase 6 - Validation And Close-Out Artifacts
|
|
|
|
Complete affected files, validation report, follow-up recommendations, and run targeted validation commands.
|
|
|
|
## Deployment / Ops Considerations
|
|
|
|
- **Environment variables**: none.
|
|
- **Migrations**: none.
|
|
- **Queues/scheduler**: none.
|
|
- **Storage/volumes**: none.
|
|
- **Filament assets**: no registered assets expected; `filament:assets` is not required unless implementation unexpectedly registers assets, which should be treated as scope drift.
|
|
- **CI**: v1 may run as a targeted guard or report-only heavy-governance check. Do not make broad heuristic failures CI-blocking before allowlist cleanup.
|
|
|
|
## Filament v5 Output Contract
|
|
|
|
- **Livewire v4.0+ compliance**: The app has Livewire 4.1.4; this spec must not introduce Livewire v3 references.
|
|
- **Provider registration location**: No panel providers are changed. If an implementation unexpectedly touches panel providers, Laravel 12 registration remains `apps/platform/bootstrap/providers.php`.
|
|
- **Global search**: No resources or global search behavior are changed.
|
|
- **Destructive actions**: No destructive/high-impact actions are added or changed. Any existing destructive actions observed by scanner remain runtime-owned and must retain confirmation, authorization, and audit behavior.
|
|
- **Asset strategy**: No new Filament assets. No deploy `filament:assets` impact expected.
|
|
- **Testing plan**: targeted Pest/source-scan guard or selected command test; no browser smoke required.
|
|
|
|
## Risk Controls
|
|
|
|
- Keep scanner deterministic and text-based.
|
|
- Default ambiguous findings to manual review.
|
|
- Keep hard failures narrow.
|
|
- Separate existing debt from new blocking failures.
|
|
- Do not auto-fix files.
|
|
- Document every allowlist entry.
|
|
- Record any chosen implementation option and rejected alternatives.
|