kjh2064
b1e38ac374
Completes VS-10/VS-12/VS-14 and makes the solution and Host actually
build and boot for the first time on this branch (main did not build
before this commit).
Root-cause fixes required to reach a green build/boot (not scoped to
J/K/L but blocking any verification of it):
- Restore Polly PackageVersion accidentally deleted from
Directory.Packages.props (broke KArtSell.Host).
- Remove MediatR dependency from Compliance/VS-04 (package was never
installed; ICommand/ICommandHandler/IMediator never existed) and
wire Endpoint -> Handler directly per this repo's convention.
- Migrate FastEndpoints v5 API calls (SendOkAsync/SendAsync/
SendCreatedAtAsync/SendNotFoundAsync, Description().WithName()) to
the v7 Send.* fluent API across ~10 endpoint files.
- Fix migrations 0036/0038/0039/0040: rewritten from invalid T-SQL
(`IF NOT EXISTS ... BEGIN ... END`) to idiomatic Postgres
(`CREATE TABLE/INDEX IF NOT EXISTS`) — these could not apply to any
fresh database before this fix.
- Collapse 3 duplicate cross-cutting abstractions that shadowed the
BuildingBlocks versions and caused type-mismatch compile errors:
IKrxDataService, IOutboxWriter (ReconcileTradeHandler), IClock
(ApprovalWorkflow/ApprovalPolicy).
- Inject IClock (BuildingBlocks.Time) in place of direct
DateTime.Now/UtcNow across 19 files to satisfy the architecture
test AGENTS.md#DateTime-abstraction rule (13/13 architecture tests
now pass, was 12/13).
- Register all new and previously-unregistered slices in
Program.cs DI (SellDecision, TradeExecution, PortfolioReconciliation,
Compliance, Features/ApprovalWorkflow) — the Host had never
successfully completed a boot with this code present.
- Disable ("[DontRegister]") the older, route-colliding
ApprovalWorkflow/ (Workstream H) endpoint set in favor of
Features/ApprovalWorkflow/ (Workstream G, matches the documented
Features/<Slice>/ convention); kept for its existing test coverage.
See TECH_DEBT-017 for the follow-up decision needed.
Verified: dotnet build 0 errors/0 warnings; architecture tests 13/13;
unit tests 54/54 + 18/18; integration tests 34/36 (2 failures are a
local test-DB migration-journal/schema mismatch, not a code defect);
Host boots cleanly and registers all 34 endpoints.
New tech debt recorded: DEBT-017 (duplicate VS-03 implementation),
DEBT-018 (outbox write not co-transactional with entity write in
TradeExecution/PortfolioReconciliation), DEBT-019 (duplicate
BuildingBlocks-shadowing abstractions, partially resolved).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
11 KiB
11 KiB
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) |
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
staticto 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
- Identify: Find in this register or add new entry with Impact/Effort estimate
- Estimate: Low (1) / Medium (2) / High (3) for each dimension
- Schedule: Pick based on matrix above
- Implement: Separate PR, reference Debt ID in commit message (e.g.,
TECH-007: Fix CA1848) - Verify: Update register (move to Completed, record date + ADR link)
- Retrospective: Review in sprint retro; aim for 20% quarterly paydown