KArtSell.Aegis/TECH_DEBT_REGISTER.md

# Tech Debt Register

**Governance:** Tracked per AGENTS.md v16.0. Quarterly paydown target: 20% by Impact.

---

## Summary

| Status | Count | Total Impact |
|--------|-------|--------------|
| Backlog | 5 | 9 pts |
| In Progress | 0 | 0 pts |
| Completed | 2 | 3 pts |
| No Action | 1 | 1 pt |
| Deferred | 5 | 7 pts |
| Accepted | 1 | 2 pts |

---

## Registry

### Code Analysis Suppressions

| ID | Category | Impact | Effort | Status | Notes | Owner | ADR |
|----|----------|--------|--------|--------|-------|-------|-----|
| DEBT-001 | CA1822 (static hints) | Low (1) | Low (1) | Completed | Applied `static` to GetNextDueAt, Evaluate, Plan methods; removed DI registrations. | @claude | PR 4b |
| DEBT-002 | CA1873 (array logging) | Low (1) | Low (1) | No Action | Already compliant: all logging uses LoggerMessage delegates. Verified PR 4b build with CA1873 enabled: 0 warnings. | @claude | Verified |
| DEBT-003 | CA1305 (culture) | Low (1) | Low (1) | Deferred | Locale-specific formatting. Accept as-is for Serilog; breaking change if fixed. Revisit if conditions change. | @claude | PR 4d |
| DEBT-004 | CA1707 (test naming) | Low (1) | Low (1) | Deferred | xUnit underscores in test names. Convention; no fix needed. Revisit if conditions change. | @claude | PR 4d |
| DEBT-005 | CA1861 (array overhead) | Low (1) | Low (1) | Deferred | Static readonly array allocations. Negligible perf; accept trade-off for readability. Revisit if conditions change. | @claude | PR 4d |
| DEBT-006 | xUnit2031 (filter) | Low (1) | Low (1) | Deferred | Use overload instead of .Where() for Assert.Single. Analyzer nit; defer. Revisit if conditions change. | @claude | PR 4d |

### Gate 3 Simplified Analytics (Deferred per v16.0)

| ID | Category | Impact | Effort | Status | Notes | Owner | ADR |
|----|----------|--------|--------|--------|-------|-------|-----|
| DEBT-009 | PBO/Sharpe calculation | High (3) | High (3) | Backlog | MetricsCalculator.cs:148,170 use simplified percentile formulas. Need proper CSCV-based PBO and DSR methodology. Required for production Sharpe baseline. Gate 3 rehearsal will use simplified version; full implementation deferred to separate work. | @claude | Gate 3 Rehearsal Scope |
| DEBT-010 | Model prediction logic | High (3) | High (3) | Backlog | ReplayEngine.cs:90,163 predict fixed quantities (100 units). Need actual position-sizing algorithm. Required for realistic cost simulation. Gate 3 uses fixed quantities; full implementation deferred. | @claude | Gate 3 Rehearsal Scope |
| DEBT-011 | Cost 2x simulation | High (3) | High (3) | Backlog | ShadowRunJob.cs:132 uses linear approximation (TotalReturn * 0.5m). Need full re-simulation with actual fee/slippage impact. Required for realistic scenario analysis. Gate 3 uses linear model; full implementation deferred. | @claude | Gate 3 Rehearsal Scope |
| DEBT-012 | False-exit analysis | High (3) | High (3) | Backlog | ShadowRunJob.cs:136-139, FalseExitAnalyzer.cs always returns 0. Unimplemented feature. Required for accurate sell-reason attribution. Gate 3 rehearsal does not include false-exit analysis; deferred to separate work. | @claude | Gate 3 Rehearsal Scope |
| DEBT-013 | Credentials in appsettings | High (3) | Low (1) | Deferred | Host/tests appsettings.json contains plaintext DB password. Deferred: not in v16.0 scope. Revisit if security compliance requirements change. | @claude | Deferred |
| DEBT-014 | Duplicate & reconciliation tracking | Medium (2) | Medium (2) | Backlog | MetricsSql.cs GetDuplicateDetectionAsync/GetReconciliationBreaksAsync return null placeholders. Requires operation_audit_trail population by job consumers + OutboxPollerJob hooks. Non-blocking; dashboard degrades gracefully. | @claude | Observability Enhancement |
| DEBT-015 | Hangfire distributed lock timeout resilience | Medium (2) | High (3) | Completed | Applied consistent try/catch(Timeout) guard to all 6 Hangfire RecurringJob registrations: line 216 (RegisterModelOperationsSchedules), 260 (OpenDartDaily), 267 (DailyRecommendation), 273 (WeeklyRecommendation), 279 (MonthlyRecommendation). Prevents silent infinite wait; logs WARN and continues if lock times out. Resolves Host startup hangs when Hangfire schema initialization contentions occur. | @claude | PR Session commit 8b1c2f1 |

### Deferred Refactoring

| ID | Category | Impact | Effort | Status | Notes | Owner | ADR |
|----|----------|--------|--------|--------|-------|-------|-----|
| DEBT-007 | Newtonsoft.Json override | Medium (2) | Medium (2) | Completed | Fixed in 88ea5ed: CA1848/CA1859 actual implementation. LoggerMessage + HashSet/Dictionary. | @claude | - |
| DEBT-008 | Namespace consistency | Medium (2) | Low (1) | Accepted | All projects use RootNamespace=KArtSell.Aegis; AssemblyName retained per-project for DLL clarity. Trade-off accepted: DLL clarity > namespace alignment. No action. | @claude | PR 4d |
| DEBT-016 | VS-02 mislabeled domain | Medium (2) | Low (1) | Backlog | Existing code `VS02_SyncSecurityMasterEndpoint.cs`, `VS02_SecurityMasterJobs.cs`, `VS02_SecurityMasterPolicy.cs` implement RBAC rule synchronization (access control), not financial security master data (listing/delisting/product structure). Dead code: endpoints disabled (DISABLED comment), schema `security_master.rules` table never migrated, never deployed. Correct domain documented in `docs/CURRENT/SLICE_SPECS/VS-02-SLICE_SPEC.md` (financial PIT). Removal decision deferred pending architect review (PR recommended). | @claude | docs/CURRENT/SLICE_SPECS/VS-02-SLICE_SPEC.md |
| DEBT-017 | Duplicate VS-03 Approval Workflow implementation | High (3) | Medium (2) | Backlog | Two independent, functionally-identical VS-03 maker-checker slices exist: `ApprovalWorkflow/` (Workstream H, own `ApprovalProposal`/`IClock`/`IOutbox` types) and `Features/ApprovalWorkflow/` (Workstream G, matches documented `Features/<Slice>/` convention). Both mapped the same routes (`/approvals`, `/approvals/{id}`, `/approvals/{id}/approve`), which crashed Host startup with a duplicate-route/missing-DI error the first time the app was actually booted (2026-08-07 — apparently never booted successfully before). Old set annotated `[DontRegister]` (FastEndpoints) 2026-08-07 to unblock boot; code and its test file (`ApprovalWorkflowTests.cs`) kept for now. Needs an architect decision: delete the old slice entirely (and its test) or intentionally keep both for a reason not yet documented. | @claude | Session 2026-08-07 (Phase 3 J/K/L hardening) |
| DEBT-018 | Outbox write not co-transactional with entity write | Medium (2) | Medium (2) | Backlog | `TradeExecution/TradeHandlers.cs` (`TradeOutboxPublisher`) and `PortfolioReconciliation/ReconcileTradeHandler.cs` open a second, separate connection/transaction to write the outbox message after the trade/holding write already committed on its own connection. A crash between the two leaves the entity updated but no outbox event emitted (silent, non-atomic). Proper fix: thread a shared `NpgsqlTransaction` through `TradeSql`/`ReconciliationSql` mutation methods so entity insert + outbox insert commit together, matching `DapperModelOperationRequestRepository`'s pattern. | @claude | Session 2026-08-07 (Phase 3 J/K/L hardening) |
| DEBT-019 | Multiple duplicate cross-cutting abstractions (`IClock`, `IOutboxWriter`, `IKrxDataService`) | Medium (2) | Low (1) | Completed (partial) | Found and collapsed 3 separate cases where a slice reinvented an abstraction that already existed in `KArtSell.BuildingBlocks`: a second `IKrxDataService` (deleted, `ShadowRun.Services`), a second `IOutboxWriter`/`WriteAsync<T>` in `ReconcileTradeHandler.cs` (removed, switched to `BuildingBlocks.Reliability.IOutboxWriter`), and a second `IClock`/`SystemClock` in `ApprovalWorkflow/ApprovalPolicy.cs` (removed, switched to `BuildingBlocks.Time.IClock`). Root cause: successive sessions implementing a slice without searching `BuildingBlocks` first. Recommend a pre-implementation checklist step ("does this abstraction already exist in BuildingBlocks?") for future slices. | @claude | Session 2026-08-07 (Phase 3 J/K/L hardening) |
| DEBT-020 | `model_operations.models` and `compliance` schema never created by any migration | High (3) | Low (1) | Completed | `0036`/`0038` reference `model_operations.models(id)` via FK and `OpenDartDailyBatchJob.cs` queries it directly, but no migration ever ran `CREATE TABLE model_operations.models`; `0037` wrote to `compliance.*` tables without `CREATE SCHEMA compliance`. Any fresh database — including the actual deploy target (178.104.200.7), confirmed via a live failed SCP/DbMigrator deploy on 2026-08-07 — failed at migration `0036`/`0037`. Fixed via new `0035_model_operations_models.sql` (minimal: id/ticker/published_at/correlation_id/revision only — full Model Card schema is separate future work) and `CREATE SCHEMA IF NOT EXISTS compliance;` added to `0037`. Full chain 0000→0040 now verified fresh-install + idempotent re-run clean. | @claude | Session 2026-08-07 (deploy failure triage) |
| DEBT-021 | Dapper never configured for snake_case↔PascalCase column mapping | High (3) | Low (1) | Completed | `Dapper.DefaultTypeMap.MatchNamesWithUnderscores` was never set anywhere in the codebase, so every `QueryAsync<T>`/`QuerySingleOrDefaultAsync<T>` result-mapping onto a snake_case DB column (e.g. `event_type` → `EventType`) silently returned null/default for that property instead of throwing — masking the bug in every Sql class across every module. Confirmed via `ApprovalWorkflowTests.InsertAndRetrieveProposal_RoundTrips` and `AuditTrailTests.InsertAuditEvent_CreatesImmutableRecord` both getting real rows back with null fields. Fixed centrally via a `[ModuleInitializer]` in `KArtSell.BuildingBlocks/Data/DapperBootstrap.cs` (runs once per process regardless of entry point — Host/DbMigrator/tests). | @claude | Session 2026-08-07 (deploy failure triage) |
| DEBT-022 | jsonb/inet columns written as plain text without an explicit cast | Medium (2) | Low (1) | Completed (partial) | Dapper does not know to cast a `string` parameter to `jsonb`/`inet` for Npgsql; `AuditSql.InsertAuditEventAsync` (`details`, `ip_address`), `AuditSql.RedactAuditEventDetailsAsync` (duplicate `SET details =` assignment, separately fixed), `TradeSql.InsertTradeAsync`/`UpdateTradeStatusAsync` (`kis_response`), and `SellDecisionSql.InsertDecisionAsync` (`oos_performance`) all failed with `42804: column "x" is of type jsonb but expression is of type text` the first time they were run against a real schema. Fixed with explicit `::jsonb`/`::inet` casts at each call site (mechanical, no behavior change). `AuditSql`'s jsonb read-back (`Dictionary<string,object>` from a jsonb column) also needed a raw-DTO + `JsonSerializer.Deserialize` mapping since Dapper has no built-in jsonb→Dictionary conversion either. **Not yet checked**: `PortfolioReconciliation`/`ApprovalWorkflow` Sql classes for the same pattern beyond what surfaced in this session's test runs — a full audit of jsonb/inet columns across all Sql classes is still open. | @claude | Session 2026-08-07 (deploy failure triage) |
| DEBT-023 | `ApprovalSql.InsertProposalAsync` fails on `DateOnly` parameter | Medium (2) | Low (1) | Backlog | `ApprovalWorkflowTests.InsertAndRetrieveProposal_RoundTrips` fails with `System.NotSupportedException: The member effectiveAt of type System.DateOnly cannot be used as a parameter value` — Dapper's `LookupDbType` doesn't recognize `DateOnly` without an explicit type map (`SqlMapper.AddTypeMap`/custom `TypeHandler`). Likely affects every other `DateOnly`-typed Dapper parameter in the codebase, not just this one; needs a similar centralized fix to DEBT-021 rather than a per-call-site patch. Discovered but not fixed in this session (scope cut to unblock the live deploy). | @claude | Session 2026-08-07 (deploy failure triage) |
| DEBT-024 | New integration tests don't insert FK parent rows / one pure-logic test flakes under full-suite run | Low (1) | Low (1) | Backlog | `TradeExecutionTests` constructs `Trade` with a random `sellDecisionId` that was never inserted into `sell_decisions`, so every insert now correctly fails its FK constraint (`trades_sell_decision_id_fkey`) once the schema was actually complete (see DEBT-020) — test-only gap, not a production code defect; needs the tests updated to insert a parent `models`+`sell_decisions` row first. Separately, `SellPriorityRankerTests.CalculateScore_HardImpairment_ReturnsLowestScore` (pure logic, no DB) passed in isolation but returned 1000 instead of the expected 950 (age-boost not applied) when run as part of the full suite — not yet root-caused; may be test-order/parallelization state leakage rather than a `SellPriorityRanker` bug. Also, `DbUpMigrationTests.*` (pre-existing, unrelated to this session) fail locally with `42501: must be owner of database kartsell_migration_test` — a local Postgres role permission gap, not a code issue. | @claude | Session 2026-08-07 (deploy failure triage) |

---

## Impact/Effort Matrix (Updated: PR 4)

```
             Low Effort    High Effort
High Impact   QUICK WINS    ROADMAP
              (DEBT-007✓)   (none currently)

Low Impact    QUICK WINS    MONITOR
              (DEBT-001/002) (DEBT-003/004/005/006/008)
```

### Quick Wins — Q3 2026 (Completed)

**Rationale (per AGENTS.md v16.0 "Paydown Target: 20% quarterly"):**
- ✅ DEBT-001 (CA1822): static method hints — Completed in PR 4b. Applied `static` to ScheduleOccurrencePlanner.GetNextDueAt, PromotionGateEvaluator.Evaluate, EvaluationWindowPlanner.Plan; removed unnecessary DI registrations (+1 pt).
- ✅ DEBT-002 (CA1873): array logging — Already compliant: all logging uses LoggerMessage delegates. Verified in PR 4b build with CA1873 enabled: 0 warnings. No action needed (+0 pts, marked "No Action").
- Result: +1 pt resolved (25% of 4pt target). Target rate achievable by completing additional small-effort items from remaining backlog.

### Batch During Feature Work
- ~~DEBT-001~~, ~~DEBT-002~~ — Moving to Quick Wins (PR 4 priority)

### Monitor & Defer (No Action)

**Rationale (per AGENTS.md "Keep in backlog; revisit if conditions change"):**

- **DEBT-003 (CA1305 culture):** Locale formatting. Accept as-is for Serilog. Breaking change risk > benefit. **Status: Permanently defer**
- **DEBT-004 (CA1707 test naming):** xUnit convention (underscores). No fix needed; convention not a defect. **Status: Permanently defer**
- **DEBT-005 (CA1861 array overhead):** Static readonly arrays. Negligible perf; readability priority. **Status: Permanently defer**
- **DEBT-006 (xUnit2031 filter):** Assert.Single overload vs .Where(). Style preference, not safety. **Status: Permanently defer**
- **DEBT-008 (Namespace consistency):** Per-project AssemblyName intentional (DLL clarity). No action needed. **Status: Accepted**

---

## Paydown Tracking

### Q3 2026 (Current)
- **Target:** 20% of total impact resolved = 4 pts
- **Completed:** DEBT-007 (2 pts) — 50% of target achieved
- **PR 4 Plan:** DEBT-001 + DEBT-002 (2 pts) — Complete 100% of target ✅
  - PR 4a: Evaluation & finalization (this commit)
  - PR 4b: DEBT-001 — CA1822 static methods implementation
  - PR 4c: DEBT-002 — CA1873 array logging optimization
  - PR 4d: Permanent defer decisions for DEBT-003~006

### Q4 2026
- **Target:** 20% = 4 pts (cumulative: 8 pts / 40% debt)
- **Plan:** TBD after Q3 completion

### Q1 2027
- **Target:** 20% = 4 pts (cumulative: 12 pts / 60% debt)
- **Plan:** TBD

---

## How to Resolve Tech Debt

1. **Identify:** Find in this register or add new entry with Impact/Effort estimate
2. **Estimate:** Low (1) / Medium (2) / High (3) for each dimension
3. **Schedule:** Pick based on matrix above
4. **Implement:** Separate PR, reference Debt ID in commit message (e.g., `TECH-007: Fix CA1848`)
5. **Verify:** Update register (move to Completed, record date + ADR link)
6. **Retrospective:** Review in sprint retro; aim for 20% quarterly paydown

---

## Related Documents

- **Governance:** [`AGENTS.md`](AGENTS.md#13-tech-debt-management-tech-debt-registry)
- **Tracking:** [`CLAUDE.md`](CLAUDE.md#tech-debt-management)
- **Decision Log:** See individual PR commit messages and ADRs