| 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) |
Completed |
✅ RESOLVED (2026-08-11 Session): Deleted all 3 dead code files: VS02_SyncSecurityMasterEndpoint.cs, VS02_SecurityMasterJobs.cs, VS02_SecurityMasterPolicy.cs. Verified: endpoints never registered (DISABLED comment in Program.cs), schema never created (no migration), neither file referenced anywhere. Removed folder src/KArtSell.Host/Features/SecurityMaster/ entirely. Build verified clean (0 errors/warnings). Rationale: pure dead code per AGENTS.md "necessity-driven" principle. |
@claude |
Session 2026-08-11 |
| 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 |
Integration test FK parent setup / SellPriorityRankerTests flaking |
Low (1) |
Low (1) |
Completed ✅ DB Verified |
✅ Code Review + DB Verified (2026-08-14): TradeExecutionTests already properly seeded — SeedSellDecisionAsync() inserts both model_operations.models and model_operations.sell_decisions rows before each test (lines 35-52), all test methods call this helper. DB Test Run 2026-08-14: dotnet test TradeExecutionTests -c Release: 13/13 PASS (67s). FK constraints verified live. All rows inserted correctly, no constraint violations. SellPriorityRankerTests: test class does not exist in codebase (stale entry). All 53 ModelOperations unit tests verified PASS in Release build. Noted: DbUpMigrationTests.* (pre-existing, unrelated) fail locally with 42501: must be owner of database kartsell_migration_test — a local Postgres role/permission gap. |
@claude |
Code audit + DB Test Pass Session 2026-08-14 |
| 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 — audit logging dead code |
High (3) |
Medium (2) |
Completed ✅ DB Verified |
✅ Wired Successfully + DB Verified (2026-08-14): AuditTrailConsumer (OutboxEventConsumer implementation) already exists and is wired into OutboxPollerJob.ExecuteAsync (line 99). Maps 11 event types (APPROVAL_PROPOSED/APPROVED/REJECTED, MODEL_ACTIVATED/DEACTIVATED, SHADOW_RUN_COMPLETED, TRADE_SUBMITTED/CONFIRMED/FAILED, SELL_DECISION_MADE/EXECUTED, RECONCILIATION_STARTED/COMPLETED) to operation_audit_trail with idempotency (ON CONFLICT DO NOTHING). Each event parsed for entity ID + correlation ID + payload JSON. Migration 0041_create_operation_audit_trail.sql schema verified (event_type, entity_type, entity_id, correlation_id, details JSONB, indexes). DB Test Run 2026-08-14: dotnet test AuditTrailTests -c Release: 5/5 PASS including GDPR redaction + retention workflows verified live. Duplicate detection via LogDuplicateDetectionAsync (logs DUPLICATE_DETECTED events separately). Production-ready. Old LogAuditEventCommandHandler remains dead code but non-breaking (marked for cleanup). |
@claude |
Verified + DB Test Pass Session 2026-08-14 |
kjh2064
60daf2c9c7