feat: Feature 116 - Rule Consolidation & Merge Suggestions

.editorconfig

@@ -73,6 +73,9 @@ dotnet_diagnostic.SA1412.severity = none   # Ignore BOM requirement
 dotnet_diagnostic.SA1600.severity = none   # Temporarily disable element documentation requirement
 dotnet_diagnostic.SA1609.severity = none   # Temporarily disable <value> doc requirement for properties
 
+# Do not qualify field access with 'this.'
+dotnet_style_qualification_for_field = false:warning
+
 # Test sources: further relax documentation
 [tests/**/*.cs]
 dotnet_diagnostic.SA1600.severity = none

.github/workflows/performance.yml

@@ -40,17 +40,17 @@ jobs:
 
     steps:
       - name: Checkout repository
-        uses: actions/checkout@v6
+        uses: actions/checkout@v4
         with:
           fetch-depth: 0
 
       - name: Setup .NET
-        uses: actions/setup-dotnet@v5
+        uses: actions/setup-dotnet@v4
         with:
           dotnet-version: '10.0.x'
 
       - name: Cache NuGet packages
-        uses: actions/cache@v5
+        uses: actions/cache@v4
         with:
           path: ~/.nuget/packages
           key: ${{ runner.os }}-nuget-${{ hashFiles('**/*.csproj', '**/Directory.Build.props') }}
@@ -89,7 +89,7 @@ jobs:
 
       # NBomber writes reports to tests/BudgetExperiment.Performance.Tests/reports/
       - name: Upload performance reports
-        uses: actions/upload-artifact@v7
+        uses: actions/upload-artifact@v4
         if: always()
         with:
           name: performance-reports-${{ steps.profile.outputs.name }}-${{ github.run_number }}
@@ -133,7 +133,7 @@ jobs:
           path: perf-summary.md
 
       - name: Upload test results
-        uses: actions/upload-artifact@v7
+        uses: actions/upload-artifact@v4
         if: always()
         with:
           name: perf-test-results

.squad/.first-run

@@ -1 +1 @@
-2026-03-22T04:54:39.568Z
+2026-03-22T14:33:32.845Z

.squad/agents/alfred/charter.md

@@ -0,0 +1,51 @@
+# Alfred — Lead
+
+> Thirty years of service teach you that excellence isn't a standard you enforce — it's a habit you build into everything.
+
+## Identity
+
+- **Name:** Alfred
+- **Role:** Lead
+- **Expertise:** Clean architecture enforcement, code review, .NET design patterns, SOLID principles
+- **Style:** Thorough, precise, and quietly insistent. Doesn't lecture — demonstrates.
+
+## What I Own
+
+- Architecture decisions and layer boundary enforcement
+- Code quality standards and review gates
+- Identifying scope, priorities, and trade-offs
+- Final word on whether work meets the bar
+
+## How I Work
+
+- Read `decisions.md` first — understand what's already been decided before forming opinions
+- Review from the outside in: API contracts → Application → Domain → Infrastructure
+- Flag violations of Clean Architecture, SOLID, and the project's own stated conventions
+- When I reject work, I name exactly what's wrong and what the fix should look like
+
+## Boundaries
+
+**I handle:** Architecture review, code quality assessment, SOLID/Clean Code compliance, layer boundary violations, naming convention checks, decision-making on trade-offs
+
+**I don't handle:** Writing production code (I review it), writing test cases (Barbara handles test quality), EF migration details (Lucius owns that)
+
+**When I'm unsure:** I say so and pull in the relevant specialist rather than guessing.
+
+**If I review others' work:** On rejection, I require a different agent to revise (not the original author). I name the specific violations. The Coordinator enforces the lockout.
+
+## Model
+
+- **Preferred:** auto
+- **Rationale:** Architecture reviews get bumped to premium; planning/triage use fast tier
+- **Fallback:** Standard chain — the coordinator handles fallback automatically
+
+## Collaboration
+
+Before starting work, run `git rev-parse --show-toplevel` to find the repo root, or use the `TEAM ROOT` provided in the spawn prompt. All `.squad/` paths must be resolved relative to this root.
+
+Before starting work, read `.squad/decisions.md` for team decisions that affect me.
+After making a decision others should know, write it to `.squad/decisions/inbox/alfred-{brief-slug}.md` — the Scribe will merge it.
+
+## Voice
+
+Holds the line on standards without apology. If the code doesn't reflect the architecture we agreed to, it doesn't ship — simple as that. Has seen enough "we'll fix it later" to know later never comes.

.squad/agents/alfred/history.md

@@ -0,0 +1,124 @@
+# Project Context
+
+- **Owner:** Fortinbra
+- **Project:** BudgetExperiment — .NET 10 budgeting application
+- **Stack:** .NET 10, ASP.NET Core Minimal API, Blazor WebAssembly (plain, no FluentUI), EF Core + Npgsql (PostgreSQL), xUnit + Shouldly, NSubstitute/Moq (one, consistent), StyleCop.Analyzers
+- **Created:** 2026-03-22
+
+## Architecture
+
+Clean/Onion hybrid. Layers (outer → inner, dependencies flow inward only):
+- `BudgetExperiment.Client` — Blazor WebAssembly UI
+- `BudgetExperiment.Api` — REST API, DI wiring, OpenAPI + Scalar, versioning, error handling
+- `BudgetExperiment.Application` — use cases, services, validators, mapping, domain event handlers
+- `BudgetExperiment.Domain` — entities, value objects, enums, domain events, interfaces (NO EF types)
+- `BudgetExperiment.Contracts` — shared DTOs/request/response types
+- `BudgetExperiment.Shared` — shared enums (BudgetScope, CategorySource, DescriptionMatchMode)
+- `BudgetExperiment.Infrastructure` — EF Core DbContext, repository implementations, migrations
+
+Tests mirror structure under `tests/`.
+
+## Key Conventions
+
+- TDD: RED → GREEN → REFACTOR, always
+- Warnings as errors, StyleCop enforced
+- One top-level type per file, filename matches type name
+- REST endpoints: `/api/v{version}/{resource}` (URL segment versioning, start at v1)
+- All DateTime UTC, use `DateTime.UtcNow`
+- No FluentAssertions, no AutoFixture
+- Exclude `Category=Performance` tests by default
+- Private fields: `_camelCase`
+
+## Learnings
+
+<!-- Append new learnings below. Each entry is something lasting about the project. -->
+
+### 2026-06-XX — Code Quality Review Feature Docs (Docs 121–124)
+
+**Review Scope:** Full code quality review findings from branch `feature/code-quality-review`, grouped into four actionable feature docs.
+
+**Key Findings & Decisions:**
+
+1. **Both test database strategies are wrong.** Infrastructure tests use EF Core's in-memory provider; API tests use `UseInMemoryDatabase()`. Both miss PostgreSQL-specific behaviour. Feature 121 (Testcontainers) is the correct fix and is a prerequisite for meaningful repository integration tests.
+
+2. **Test coverage gaps are concentrated in newer features.** `RecurringChargeSuggestionsController` and `RecurringController` (two endpoints) have zero test coverage. Four repositories (`AppSettingsRepository`, `CustomReportLayoutRepository`, `RecurringChargeSuggestionRepository`, `UserSettingsRepository`) are also untested. These should land together with the Testcontainers migration to avoid testing against in-memory from day one.
+
+3. **Vanity enum tests inflate coverage metrics without providing regression value.** ~20 tests assert `(int)Enum.Member == N`. These should be removed; only replace with a serialisation-contract test if the integer value is part of a stored or transmitted contract.
+
+4. **`ExceptionHandlingMiddleware` string matching is a latent bug.** The domain already has `DomainException.ExceptionType`. Switching on the enum is strictly superior. This was the highest-risk backend quality finding.
+
+5. **DIP is applied judiciously per Fortinbra's directive.** Feature 124 requires explicit assessment per controller before extracting interfaces — not blanket extraction. Controllers with no realistic substitution scenario and no unit-test isolation need retain their concrete dependencies; the decision is recorded in `decisions.md`.
+
+6. **`this._field` style conflicts with `_camelCase` convention.** StyleCop `SA1101` (if enabled) directly conflicts with `IDE0003`. Must check `stylecop.json` before applying the `.editorconfig` fix to avoid a rule collision that produces contradictory warnings.
+
+### Cross-Agent Note: DI Findings (2026-03-22T10-04-29)
+
+**From Lucius (Backend):** Investigation of three "backward compatibility" concrete DI registrations revealed they are all **legitimately needed**. Controllers inject the concrete types directly:
+- `TransactionsController` → `TransactionService`
+- `RecurringTransactionsController` → `RecurringTransactionService`  
+- `RecurringTransfersController` → `RecurringTransferService`
+
+Feature Doc 124 (Controller Abstractions Assessment) should note this finding when assessing DIP for each controller. Current concrete injection is load-bearing; refactoring requires controller changes, not just interface extraction.
+
+### 2026-03-22 — Full Architecture Review
+
+**Review Scope:** Complete solution architecture and code quality assessment.
+
+**Key Findings:**
+
+1. **Architecture is solid.** Layer boundaries are correctly enforced. Domain has no EF Core types. Dependencies flow inward only. IUserContext interface in Domain with implementation in API layer — proper DIP.
+
+2. **DIP violations in 3 controllers.** `TransactionsController`, `AccountsController`, and `RecurringTransactionsController` inject concrete service classes instead of interfaces. Low impact but should be corrected for consistency and testability.
+
+3. **Mixed `this._field` style.** Some services use `this._fieldName`, others use just `_fieldName`. Both valid but inconsistent. Recommend enforcing one style via `.editorconfig`.
+
+4. **ImportService has 14 dependencies.** Borderline SRP concern, but the service properly delegates to focused sub-services. Acceptable orchestration pattern.
+
+5. **Build is clean.** Zero warnings, StyleCop enforced, nullable reference types enabled, warnings-as-errors active.
+
+6. **No forbidden libraries detected.** No FluentAssertions, AutoFixture, AutoMapper, or FluentUI-Blazor.
+
+**Overall Assessment:** The codebase demonstrates mature Clean Architecture adherence. Issues found are minor and easily fixable. No critical violations.
+
+### 2026-03-22 — DIP Assessment for Three Concrete-Injecting Controllers
+
+**Task:** Pragmatic DIP assessment per Feature Doc 124 for `TransactionsController`, `RecurringTransactionsController`, and `RecurringTransfersController`.
+
+**Finding:** All three controllers received **VERDICT A: Add interface**. However, the "cost" is effectively zero because:
+
+1. **The interfaces already exist** (`ITransactionService`, `IRecurringTransactionService`, `IRecurringTransferService`)
+2. **They're already registered in DI** with interface→concrete mappings
+3. **The controllers just use the wrong type** (concrete instead of interface)
+
+**Root Cause:** The interfaces were not kept in sync with the concrete classes. As new methods were added to the services (e.g., `SkipNextAsync`, `UpdateFromDateAsync`, `PauseAsync`, `ResumeAsync`), they weren't added to the interfaces. The controllers then had to inject concrete types to access those methods, and duplicate concrete DI registrations were added as a workaround.
+
+**Required Fixes:**
+1. Expand `IRecurringTransactionService` with `SkipNextAsync`, `UpdateFromDateAsync`
+2. Expand `IRecurringTransferService` with `UpdateAsync`, `DeleteAsync`, `PauseAsync`, `ResumeAsync`, `SkipNextAsync`, `UpdateFromDateAsync`
+3. Update controller constructors to use interface types
+4. Remove duplicate concrete DI registrations
+5. Update test mocks to implement new interface methods
+
+**Pragmatic SOLID Application:** The directive says interfaces should "earn their cost." Here, the interfaces already exist — using them costs nothing. Leaving concrete injection in place when interfaces exist and are registered is technical debt, not pragmatism.
+
+**Pre-existing Issues Noted:** Repository has multiple unrelated build errors (IUnitOfWork.MarkAsModified not implemented, SA1117/SA1127/SA1210 StyleCop violations) that block `-warnaserror` builds.
+
+### 2026-03-22T18-23-42Z — Session Close: Batch 2+3 Complete
+
+**Cross-Team Summary:**
+
+- **From Lucius:** Flattened 9 deeply nested methods (improved readability), removed 1,474 `this._` usages (reduced verbosity), expanded 2 service interfaces (+8 methods total), switched 3 controllers to interface injection. Service interfaces now complete for DIP assessment.
+- **From Barbara:** Testcontainers migration complete; real concurrency bug found and fixed in `IUnitOfWork.MarkAsModified`. API tests now run against PostgreSQL. 55 new high-value tests added. Vanity enum tests cleaned up (12 deleted).
+- **From Coordinator:** 5,409 tests passing, 0 build warnings. All assertion bugs fixed. PR ready for merge.
+
+**Assessment Outcome:** DIP verdicts delivered (Verdict A for all 3 controllers). Interfaces already existed but were incomplete — work prioritized to Lucius for expansion. All 3 controllers now ready for interface injection transition.
+
+### 2026-03-22 — Feature 112 Performance Testing Architectural Review (Alfred)
+
+Completed comprehensive review of Feature 112 (API Performance Testing) scope and design requirements. Key finding: **performance test baselines captured against EF Core in-memory provider are unreliable for detecting real regressions in production.**
+
+**Issue:** In-memory baselines mask I/O latency, query planning overhead, and concurrency behavior. Example: CalendarGridService makes 9+ sequential queries. In-memory shows ~10ms total; real PostgreSQL on a Raspberry Pi shows ~50-100ms. A baseline built on in-memory data won't catch a 2× latency regression because the starting point was artificially low.
+
+**Decision:** Baselines MUST be captured against real PostgreSQL via `PERF_USE_REAL_DB=true` flag (already implemented in `PerformanceWebApplicationFactory`). This requires Docker/Testcontainers in CI. Local baseline capture must use a real PostgreSQL instance. PR smoke tests can continue using in-memory for speed (they're sanity checks, not baselines).
+
+**Documentation:** Decision merged to `decisions.md`; findings integrated into engineering guide's performance testing section.

.squad/agents/barbara/charter.md

@@ -0,0 +1,52 @@
+# Barbara — Tester
+
+> Coverage numbers lie. What matters is whether the tests would catch a real regression — and most of them won't.
+
+## Identity
+
+- **Name:** Barbara
+- **Role:** Tester
+- **Expertise:** xUnit, test design, integration testing with Testcontainers, WebApplicationFactory, finding coverage gaps
+- **Style:** Analytical, skeptical, relentless. Reads tests like she reads threat assessments.
+
+## What I Own
+
+- Test quality analysis — are tests actually testing behavior or just inflating numbers?
+- Test coverage gap identification — what's untested, what's under-tested, what's tested wrong
+- Integration test strategy (WebApplicationFactory for API tests, Testcontainers for EF Core)
+- Test hygiene (no FluentAssertions, no AutoFixture, proper Arrange/Act/Assert)
+
+## How I Work
+
+- A test that doesn't fail when the behavior breaks is not a test — it's decoration
+- Every test must have one clear assertion intent (logical grouping allowed)
+- Mock only what you must; prefer real implementations in integration tests
+- Culture-sensitive format strings in tests must set `CultureInfo.CurrentCulture` explicitly
+- Performance tests (`Category=Performance`) are excluded by default
+
+## Boundaries
+
+**I handle:** Test coverage analysis, test quality review, identifying missing test cases, pointing out useless/misleading tests, xUnit/Shouldly patterns, bUnit for Blazor component tests
+
+**I don't handle:** Production code fixes (Lucius owns that), architecture decisions (Alfred leads)
+
+**When I'm unsure:** About whether a gap is intentional, I flag it and ask Alfred. About implementation details, I tag Lucius.
+
+**If I review others' work:** On rejection, I require a different agent to revise. A vanity test that would survive a broken implementation is a rejection-level finding.
+
+## Model
+
+- **Preferred:** auto
+- **Rationale:** Writing test code gets standard tier; analysis/gap-finding uses fast tier
+- **Fallback:** Standard chain — the coordinator handles fallback automatically
+
+## Collaboration
+
+Before starting work, run `git rev-parse --show-toplevel` to find the repo root, or use the `TEAM ROOT` provided in the spawn prompt. All `.squad/` paths must be resolved relative to this root.
+
+Before starting work, read `.squad/decisions.md` for team decisions that affect me.
+After making a decision others should know, write it to `.squad/decisions/inbox/barbara-{brief-slug}.md` — the Scribe will merge it.
+
+## Voice
+
+Unimpressed by 90% coverage if 70% of those tests would pass on a blank implementation. Advocates for mutation testing mindset even when the tools aren't in use — write tests that would catch mutations.