kjh2064
9ffb740f07
Systematic sweep of every *Handler registered in Program.cs (same
method that found DEBT-026/027) found ActivateModelHandler was the
last orphan in Features/ApprovalWorkflow/: no POST /approvals/{id}/activate
endpoint existed, so an Approved proposal could never reach Active -
the entire point of this maker-checker slice.
While wiring it up, found the handler's original call would have
overwritten the checker's approved_by/approval_notes with the
activating SRE's identity (it passed userEmail through
UpdateProposalStatusAsync's approvedBy parameter), and never set
activated_by/activated_at at all despite those columns existing since
migration 0036. Added a dedicated ApprovalWorkflowSql.ActivateProposalAsync
that only touches activation-specific columns, and a regression test
asserting the checker's approval record survives activation unchanged.
Also documents DEBT-029 (discovered, not fixed - genuine cross-cutting
scope): LogAuditEventCommandHandler is never called by any other
slice, so VS-27's audit trail is empty in production regardless of
activity even though its own tests pass. Downgraded AEG-VS-27-01 from
COMPLETED to BLOCKED in the tracker to reflect that honestly.
dotnet build KArtSell.sln -c Release: clean. Not run against a live
database this session.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
28 KiB
28 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-26 (formerly VS-03) Approval Workflow implementation | High (3) | Medium (2) | Completed (DB verification pending) | Decision (2026-08-08): Features/ApprovalWorkflow/ (Workstream G) kept as canonical — it is the implementation actually wired into Program.cs/FastEndpoints. src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ (Workstream H, [DontRegister]'d dead code) and its dedicated test file (tests/KArtSell.Integration.Tests/ApprovalWorkflow/ApprovalWorkflowTests.cs, the old 20/20-passing suite that exercised only the dead code) were deleted. ApprovalWorkflowPolicyTests.cs already tested the kept implementation's pure Policy class and was extended (5→10 cases) rather than replaced. New Handler+Sql+real-Postgres integration tests were written at the same path the old dead-code tests occupied (tests/KArtSell.Integration.Tests/ApprovalWorkflow/ApprovalWorkflowTests.cs), covering create (Maker-role-gated), approve (Maker≠Checker separation of duties, Checker-role-gated, evidence attachment), activate (SRE-role-gated), list filtering, and an explicit DateOnly EffectiveAt round-trip. Bug found and fixed while porting: the kept implementation's Sql.cs InsertProposalAsync had the exact same Dapper-cannot-bind-DateOnly bug that was found and fixed in the deleted implementation's ApprovalSql.cs (commit 2ccf74c) — i.e. the "tested" dead code had already been fixed for this, but the "live" code had not; it would have failed 100% of proposal-creation calls against a real database. Fixed identically (::date cast + "yyyy-MM-dd" string parameter). Not fixed (out of scope, flagged as residual gaps in the slice's README): no GET /approvals/{id} endpoint (evidence becomes unreachable via HTTP after approval), and no wired Draft→Proposed transition anywhere in the running app (ApprovalWorkflowPolicy.CanProposeForReview exists but no Handler/Endpoint calls it), and approval_proposals rows are mutated in place via UPDATE rather than appended as new PIT revisions (the table's schema only has id as PRIMARY KEY, so the deleted implementation's append-only INSERT approach would itself have violated that constraint on the second write — this is pre-existing, schema-level, and not a regression from this cleanup). Verification status: dotnet build -c Release is clean (0 errors/warnings). dotnet test --filter "FullyQualifiedName~ApprovalWorkflow" -c Release was run 2026-08-08: 10/10 pure-Policy tests passed; all 8 new DB-backed integration tests failed with Npgsql.NpgsqlException: Failed to connect to 127.0.0.1:5432 (connection refused) because no PostgreSQL was reachable in that session (no SSH tunnel to 178.104.200.7 open). None of the 8 have been confirmed to pass against a real database. Do not mark this row fully verified until that run happens; see docs/CURRENT/CATALOGS/WBS_PROGRESS_TRACKER.csv row AEG-VS-26-01, kept BLOCKED for the same reason. |
@claude | commit a2e742c (original dup.), this session's commit (resolution), docs/DECISIONS/ADR-WBS-001-slice-renumbering.md |
| DEBT-018 | Outbox write not co-transactional with entity write | Medium (2) | Medium (2) | Completed (DB verification pending) | Fixed 2026-08-08, matching DapperModelOperationRequestRepository's pattern. TradeExecution: added a DbConnection/DbTransaction-taking overload of ITradeSql.UpdateTradeStatusAsync; TradeOutboxPublisher.PublishAsync replaced with UpdateAndPublishAsync, which opens one connection/transaction, updates trade status and writes the outbox message on it, then commits once — used by all 3 call sites that publish an event (SubmitTradeHandler, the FullyFilled branch of PollTradeStatusHandler, ConfirmSettlementHandler); paths with no outbox event still use the plain non-transactional update. PortfolioReconciliation: ReconcileTradeHandler now injects the request-scoped IDbConnection (the same instance ReconciliationSql already uses within one HTTP request, replacing its own separate IDbConnectionFactory-opened connection) and begins one IDbTransaction shared by ReconciliationEngine.ReconcileTradeAsync(..., transaction) (which threads it into new IDbTransaction-aware overloads of GetHoldingAsync/UpsertHoldingAsync/InsertReconciliationLogAsync — the read needed a transaction-aware overload too, since Npgsql throws if a command on a connection with a pending transaction doesn't have it attached) and the outbox TradeReconciled/ReconciliationMismatchAlert writes; the handler commits once at the end (or rolls back on !result.Success). dotnet build KArtSell.sln -c Release: 0 warnings/0 errors. dotnet test --filter "FullyQualifiedName~TradeExecution|FullyQualifiedName~PortfolioReconciliation" -c Release: 17 pure-logic tests passed, 13 DB-backed tests failed with the same pre-existing 127.0.0.1:5432 connection-refused error (no SSH tunnel in this session) — none of the transactional changes have been confirmed against a live database yet. |
@claude | Session 2026-08-07 (Phase 3 J/K/L hardening, discovery), Session 2026-08-08 (fix) |
| 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 | 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. 2026-08-09: full audit completed (repo-wide, not just Portfolio/Approval). Enumerated every jsonb/inet column across db/migrations/*.sql (case-insensitive — several use JSONB/INET uppercase, which an earlier lowercase-only grep would have missed), then checked each one for a C# writer. Findings: PortfolioReconciliation's tables (portfolio_management.holdings/reconciliation_logs) have no jsonb/inet columns at all — nothing to fix. ApprovalWorkflow's one jsonb column (approval_events.details) was already cast correctly in InsertEventAsync. Several other jsonb columns (evidence_snapshot.payload, execution-assurance/model-feedback tables under evaluation/governance) have no C# writer yet at all — those slices (VS-05/09/19 etc.) are unimplemented, so there's no bug surface yet; flag for re-check whenever they get built. One new, real instance of this exact bug found and fixed: OpenDartService.CacheResultAsync (src/KArtSell.Host/Observability/OpenDartService.cs) inserted a serialized JSON string into opendata.opendart_cache.data_json JSONB without a cast — same 42804 failure mode as the others, just never previously exercised/caught. Fixed with @dataJson::jsonb. dotnet build -c Release clean; not run against a live database this session (see the rest of this session's entries for why). |
@claude | Session 2026-08-07 (deploy failure triage, discovery), Session 2026-08-09 (full audit + OpenDartService fix) |
| DEBT-023 | ApprovalSql.InsertProposalAsync fails on DateOnly parameter |
Medium (2) | Low (1) | Completed | Stale entry, corrected 2026-08-08: this described ApprovalSql.cs under src/KArtSell.Modules.ModelOperations/ApprovalWorkflow/ — that per-call-site fix (::date cast + "yyyy-MM-dd" string parameter, not a centralized type handler) landed in commit 2ccf74c but this row was never updated to reflect it. That whole file was then deleted as dead code while resolving DEBT-017 (2026-08-08); its surviving sibling, Features/ApprovalWorkflow/Sql.cs, was found to have the same unfixed bug independently and received the identical fix in that session — see DEBT-017. No centralized DateOnly type handler was added; this remains a per-call-site fix pattern, so any other DateOnly-typed Dapper INSERT elsewhere in the codebase should still be checked individually rather than assumed safe. |
@claude | commit 2ccf74c; DEBT-017 (this session) |
| 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) |
| DEBT-025 | Features/ApprovalWorkflow has no GET /approvals/{id} endpoint |
Medium (2) | Low (1) | Completed (DB verification pending) | Added GetApprovalByIdEndpoint (GET /approvals/{id}) + ApprovalDetailResponse (includes Evidence), and ApprovalWorkflowSql.GetEvidenceForProposalAsync. Evidence attached during approval (PBO/DSR/OOS artifact links) is now readable via HTTP. Two new tests added (GetEvidenceForProposalAsync_ReturnsEvidenceAttachedDuringApproval + the endpoint itself). dotnet build -c Release clean (0/0). Not verified against a live database — same 127.0.0.1:5432 connection-refused blocker as DEBT-017/026; do not mark fully verified until a real Postgres run passes. |
@claude | DEBT-017 (2026-08-08), src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/README.md |
| DEBT-026 | Features/ApprovalWorkflow has no wired Draft→Proposed transition |
High (3) | Low (1) | Completed (DB verification pending) | Added ProposeForReviewHandler + POST /approvals/{id}/propose, wired into Program.cs DI. Calls the pre-existing ApprovalWorkflowPolicy.CanProposeForReview (creator-only) and ValidateProposalState (Draft→Proposed), then updates status and emits a PROPOSED event — same pattern as ApproveApprovalHandler/ActivateModelHandler. A proposal created via POST /approvals can now reach Approved/Active through the HTTP API end-to-end. Two new tests added (ProposeForReview_ByCreatingMaker_TransitionsDraftToProposed, ProposeForReview_ByDifferentUserThanCreator_ThrowsUnauthorized). dotnet build -c Release clean (0/0). Not verified against a live database — same 127.0.0.1:5432 connection-refused blocker as DEBT-017/025; dotnet test --filter FullyQualifiedName~ApprovalWorkflowTests -c Release run 2026-08-08, all 17 matched tests fail with connection-refused (includes this file's tests plus an unrelated top-level ApprovalWorkflowTests.cs the substring filter also matches). Do not mark fully verified until a real Postgres run passes. |
@claude | DEBT-017 (2026-08-08), src/KArtSell.Modules.ModelOperations/Features/ApprovalWorkflow/README.md |
| DEBT-027 | PollTradeStatusHandler/ConfirmSettlementHandler registered in DI but never invoked by anything |
High (3) | Low (1) | Completed (DB verification pending) | Discovered while looking for BE/scheduler priority work (2026-08-09) — same class of gap as DEBT-026 (a fully-implemented handler with no caller). TradeEndpoints.cs only has POST /trades (→SubmitTradeHandler) and GET /trades; nothing ever called PollTradeStatusHandler or ConfirmSettlementHandler, and no Hangfire job did either, so a trade could reach Submitted and never progress — KIS fills and settlement confirmations were never picked up. Added src/KArtSell.Host/Jobs/TradeStatusPollingJob.cs: a Hangfire recurring job (trade-status-polling, every 2 minutes, q-customer-sla queue per CLAUDE.md's queue-isolation guidance since this affects real trade completion, not research) that queries Submitted/Accepted/PartiallyFilled trades and calls PollTradeStatusHandler, then queries FullyFilled trades and calls ConfirmSettlementHandler. Registered in Program.cs alongside the other recurring jobs. dotnet build -c Release clean (0/0). No dedicated test added (the job is thin orchestration over the already-implemented, already-covered-elsewhere handlers, and writing a fake IKisTradeExecutionService/ITradeSql test double would be a new testing pattern not used anywhere else in this codebase — flagged rather than done rashly) and not run against a live database or KIS — same connection blocker as the rest of this session's work. |
@claude | Session 2026-08-09 (BE/scheduler priority pass) |
| DEBT-028 | ActivateModelHandler had no HTTP endpoint, and would have corrupted approval data if wired naively |
High (3) | Low (1) | Completed (DB verification pending) | Found via a systematic sweep of every *Handler registered in Program.cs's DI container, checking whether each is actually referenced by an Endpoint.cs or a job (the same method that found DEBT-026/027) — ActivateModelHandler was the only remaining orphan in Features/ApprovalWorkflow/: no POST /approvals/{id}/activate existed, so an Approved proposal could never reach Active, the step this whole slice exists for. While wiring it up, found the handler's original call — _sql.UpdateProposalStatusAsync(proposalId, ApprovalStatus.Active, userEmail, "Model activated by SRE", ct) — would have passed the activating SRE's email/note through the approvedBy/approvalNotes parameters, overwriting the checker's real approved_by/approval_notes on activation, and never touched the schema's activated_by/activated_at columns at all (they existed since migration 0036 but nothing ever wrote them). Added a dedicated ApprovalWorkflowSql.ActivateProposalAsync(proposalId, activatedBy, ct) that only sets status='ACTIVE', activated_by, activated_at, leaving approved_by/approval_notes untouched, and switched ActivateModelHandler to call it. Added ActivateApprovalEndpoint (POST /approvals/{id}/activate). Strengthened the existing Activate_BySreAfterApproval_TransitionsToActive test to assert activated_by/activated_at are set and the checker's approved_by/approval_notes survive activation unchanged — this would have caught the bug. dotnet build -c Release clean (0/0). Not run against a live database this session. |
@claude | Session 2026-08-09 (BE/scheduler priority pass) |
| DEBT-029 | LogAuditEventCommandHandler (VS-27 audit trail) is never called by any other slice |
High (3) | Medium (2) | Backlog | Found during the same orphaned-handler sweep that found DEBT-027/028 — but unlike those, this one is NOT a missing single endpoint; it's a missing cross-cutting integration. LogAuditEventCommandHandler (src/KArtSell.Modules.ModelOperations/Compliance/LogAuditEventHandler.cs) is the intended call point for every other slice to record an auditable action (per compliance.audit_event_types's seed data: APPROVAL_PROPOSED, APPROVAL_APPROVED, MODEL_ACTIVATED, SELL_DECISION_MADE, SELL_EXECUTED, etc.) — but nothing in ApprovalWorkflow/Handlers.cs, TradeExecution/TradeHandlers.cs, SellDecision/*, or PortfolioReconciliation/ReconcileTradeHandler.cs actually calls it. VS-27 ("Immutable Audit Trail") is marked COMPLETED in the WBS tracker with 5/5 tests passing, but those tests only exercise AuditSql directly — they don't prove the rest of the system ever produces an audit trail in practice. Net effect: the compliance/GDPR audit trail this system's governance model depends on (CLAUDE.md's "Evidence & Audit: Update/delete are blocked; new state appended as new revision") is currently empty in production regardless of how many approvals/trades/sell-decisions happen, because nothing populates it outside of direct AuditSql test calls. Not fixed this session — wiring it in touches 4+ handler classes across 3+ slices (a genuine cross-cutting integration, not a single bounded fix like DEBT-026/027/028), and each call site needs to decide what EventType/Details/EvidenceLinks are correct for that action rather than a mechanical change. Recommend one slice at a time, starting with ApprovalWorkflow (highest governance stakes) via its outbox events (ApprovalWorkflowPolicy.CreateStateChangeEvent already emits an event per transition — a downstream consumer job could call LogAuditEventCommandHandler from there instead of wiring it into every handler directly, matching this repo's existing Outbox→Inbox→consumer pattern). |
@claude | Session 2026-08-09 (BE/scheduler priority pass, discovery only) |
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