dd352596fce8140a5474480f1c56c2caf46c0aaa
6 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 kjh2064
|
8ed232e224 |
fix: DEBT-018 - make outbox writes co-transactional with entity writes
TradeExecution: 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 the 3 call sites that publish an event after a status update (SubmitTradeHandler, PollTradeStatusHandler's FullyFilled branch, ConfirmSettlementHandler). PortfolioReconciliation: ReconcileTradeHandler now injects the request-scoped IDbConnection (the same instance ReconciliationSql already uses) instead of opening a second separate connection via IDbConnectionFactory, begins one transaction shared by ReconciliationEngine.ReconcileTradeAsync and the outbox writes, and commits once. Required adding IDbTransaction-aware overloads of GetHoldingAsync/UpsertHoldingAsync/InsertReconciliationLogAsync - the read needed one too, since Npgsql throws if a command on a connection with a pending transaction doesn't have it attached. dotnet build KArtSell.sln -c Release: clean. Integration tests: 17 pure-logic tests pass, 13 DB-backed tests fail with the pre-existing connection-refused error (no SSH tunnel in this environment) - the transactional changes themselves are not yet verified against a live database. Also corrected two WBS_PROGRESS_TRACKER.csv rows that inaccurately said frontend UI work was "in progress" when the background agents building VS-28/VS-29 UI had actually failed (hit the session's spend limit) before committing anything. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
kjh2064
|
60c90baa28 |
docs: resolve VS-03/04/12/14 numbering collision (renumber to VS-26/27/28/29)
Renumbers the four 2026-08-07 slices (ApprovalWorkflow, AuditTrail, TradeExecution, PortfolioReconciliation) to previously-unused VS-26..29, leaving WBS_MASTER.csv's original VS-03/04/12/14 definitions (IngestMarketDataPIT, ApplyCorporateActions, RankBuyCandidates, GenerateDailyRecommendations) untouched, per docs/DECISIONS/ADR-WBS-001-slice-renumbering.md. While investigating, found two things not yet resolved by this commit: - DEBT-017 (duplicate ApprovalWorkflow implementation): the tested backend (ApprovalWorkflow/) is [DontRegister]'d dead code; the live one (Features/ApprovalWorkflow/, wired in Program.cs) has no dedicated tests. AEG-VS-26-01 downgraded from COMPLETED to BLOCKED in the tracker pending an architect decision on which implementation is canonical. - Features/MarketData and Features/Portfolio (VS-03/04/05/08 Market Data Ingestion Dashboard, Portfolio Rebalance, Risk Metrics, Dashboard) are a third, already-implemented-and-tested body of work entirely absent from WBS_PROGRESS_TRACKER.csv. Flagged in CURRENT_ROADMAP.md as a follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
kjh2064
|
8ed2bcf56f |
fix: Dapper underscore-mapping race condition affects AuditSql too, not just TradeSql
The static-ctor guard added to TradeSql in the previous commit was a symptom fix. Confirmed the same bug independently affects AuditSql: running the Compliance test filter in isolation (no other class that happens to touch a BuildingBlocks type first) reproduced the identical failure mode - every snake_case column (event_type, purge_status, ...) silently mapped to null. Root cause: KArtSell.BuildingBlocks.Data.DapperBootstrap's [ModuleInitializer] only runs once that assembly is actually loaded, and a `using` directive for a BuildingBlocks namespace does not force that load - only an executed reference to one of its types does. Any Sql class that never actually touches a BuildingBlocks type at runtime is exposed, and this is a property of *when* a given test/request happens to run relative to everything else in the process, not of any one class. Replaced the ad-hoc TradeSql static ctor with one [ModuleInitializer] per module assembly (KArtSell.Modules.ModelOperations, KArtSell.Modules.SignalEngine). Every Sql/reader class lives inside its own module's assembly, so a module initializer there is guaranteed to run before any of them are used, independent of BuildingBlocks or load order. Verified both KArtSell.Integration.Tests.Compliance and .TradeExecution now pass 100% run in full isolation, not just as part of the full suite. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> |
||
|
kjh2064
|
2ccf74c410 |
fix: Release build breakage + Dapper mapping bugs in VS-03/VS-04/Phase3-K
- KArtSell.Host.csproj: FrontendFiles glob was evaluated at project-load time, before pnpm build ran, so it copied stale/missing Vite-hashed filenames every Release build. Move the glob inside the target, after the build Exec. - ApprovalSql/AuditSql/TradeSql: fix live-DB integration failures never caught by unit tests: DateOnly and inet columns can't be bound/read directly through Dapper without conversion; kis_response (jsonb) read as JsonElement threw InvalidCastException; GdprRetention.RetentionEndsAt was typed DateTime against a DATE column. - TradeSql: UpdateTradeStatusAsync only ever persisted status/kis_response /error_message, silently dropping kis_order_id, executed_quantity, unit_price, total_amount, commission, net_proceeds and the execution/ settlement timestamps on every call. Changed it to take the Trade aggregate so the full state transition persists. - TradeSql: add a static ctor setting Dapper.DefaultTypeMap. MatchNamesWithUnderscores = true. The repo's [ModuleInitializer] in KArtSell.BuildingBlocks only fires once that assembly is actually loaded; TradeSql/Trade never reference a BuildingBlocks type, so under test isolation (or any host that queries a trade before touching BuildingBlocks) every snake_case column silently mapped to null/default. - Test fixes: seed the FK prerequisites (model_operations.models, sell_decisions) that ApprovalWorkflowTests/TradeExecutionTests were missing, correct a SellPriorityRanker test input to match the approved VS-10-SLICE_SPEC age-boost threshold, and fix a GDPR redaction assertion that called ToString() on a Dictionary instead of inspecting its values. 12 DbUpMigrationTests failures remain and are unrelated to this fix: the kartsell DB user isn't the owner of kartsell_migration_test, so DbUp's fresh-database rehearsal can't DROP/CREATE it. Needs a DBA grant. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> |
||
|
kjh2064
|
4059828abf |
fix: missing model_operations.models table + compliance schema breaking every fresh DB (live deploy failure)
The SCP/DbMigrator deploy to the production target (178.104.200.7)
failed today with `relation "model_operations.models" does not
exist` at migration 0036 — the exact same failure I'd already hit
against a local test database, confirming this isn't environment
drift but a real, deterministic bug: no migration ever created
model_operations.models, only referenced it via FK (0036, 0038) and
queried it directly (OpenDartDailyBatchJob.cs). Migration 0037 had
the same class of bug for the `compliance` schema itself.
- Add 0035_model_operations_models.sql (must sort before 0036).
Scope is intentionally minimal — id/ticker/published_at/
correlation_id/revision, i.e. only what's actually referenced
today. The full Model Card/lifecycle schema is separate, larger
work and isn't guessed at here.
- Add `CREATE SCHEMA IF NOT EXISTS compliance;` to 0037, plus
IF NOT EXISTS on its indexes for re-run idempotency (matching the
rest of this migration set).
- Verified: full chain 0000->0040 applies to a fresh DB
("Upgrade successful") and re-run is a clean no-op
("No new scripts need to be executed").
Fixing the schema far enough to actually run queries against it
surfaced 3 more real, previously untested bugs in already-merged
code (none reachable before because the tables/schema didn't exist):
- Dapper was never configured for snake_case<->PascalCase column
mapping (`Dapper.DefaultTypeMap.MatchNamesWithUnderscores`), so
every Sql class's result-set queries were silently returning
null/default for every property instead of throwing. Fixed once,
centrally, via a `[ModuleInitializer]` in
KArtSell.BuildingBlocks/Data/DapperBootstrap.cs so it's set before
the first query regardless of entry point (Host/DbMigrator/tests).
- jsonb/inet columns written without an explicit cast
(`42804: column "x" is of type jsonb but expression is of type
text`) in AuditSql (details, ip_address), TradeSql (kis_response),
SellDecisionSql (oos_performance) — fixed with `::jsonb`/`::inet`
casts. AuditSql's jsonb read-back into Dictionary<string,object>
also needed a raw-DTO + JsonSerializer.Deserialize mapping.
- AuditSql.RedactAuditEventDetailsAsync had a literal duplicate
`SET details = ... details = ...` (invalid SQL) — nested the two
jsonb_set calls into one assignment.
Verified: dotnet build 0/0; architecture 13/13; unit 54/54+18/18;
Host boots cleanly and registers all 34 endpoints against the
now-complete schema.
New tech debt recorded: DEBT-020 (this fix), DEBT-021 (Dapper
snake_case fix), DEBT-022 (jsonb/inet casts, partial — not yet
audited beyond what surfaced), DEBT-023 (ApprovalSql.
InsertProposalAsync still fails on a raw DateOnly parameter — same
class of issue as DEBT-021, not yet fixed), DEBT-024 (TradeExecution
tests don't insert FK parent rows; one pure-logic ranker test
returned 1000 instead of 950 under the full suite, not yet
root-caused; DbUpMigrationTests fail locally on a Postgres role
permission gap unrelated to this fix).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
||
|
kjh2064
|
b1e38ac374 |
feat: Phase 3 J/K/L (Sell Decision, Trade Execution, Portfolio Reconciliation) + fix pre-existing build/boot breakage
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>
|