kjh2064/KArtSell.Aegis
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: Release build breakage + Dapper mapping bugs in VS-03/VS-04/Phase3-K #30

Merged
kjh2064 merged 2 commits from fix/dapper-underscore-mapping-and-build into main 2026-08-07 23:47:41 +09:00
Conversation 0 Commits 2 Files Changed 13
+155 -92
kjh2064 commented 2026-08-07 23:46:26 +09:00
Owner
Copy Link
Copy Source

Summary

  • KArtSell.Host.csproj: the FrontendFiles glob was evaluated at project-load time, before pnpm build ran, so dotnet build -c Release copied stale/missing Vite-hashed filenames on every build. Fixed by moving the glob inside the target, after the build step.
  • Fixed 4 real Dapper mapping bugs never caught by unit tests, all in code merged without ever running against a live database: DateOnly params in ApprovalSql (VS-03), inet/jsonb column reads in AuditSql (VS-04), GdprRetention.RetentionEndsAt typed DateTime against a DATE column, and TradeSql.UpdateTradeStatusAsync silently dropping kis_order_id/executed_quantity/unit_price/commission/net_proceeds/timestamps on every call (Phase 3 K).
  • Root-caused and fixed a process-wide race condition: Dapper's snake_case-to-PascalCase column mapping is set by a [ModuleInitializer] in KArtSell.BuildingBlocks that only fires once that assembly is actually loaded. TradeSql/AuditSql don't reliably touch a BuildingBlocks type at runtime, so depending on unrelated test/host startup order, every snake_case column could silently map to null. Replaced with one [ModuleInitializer] per module assembly (KArtSell.Modules.ModelOperations, KArtSell.Modules.SignalEngine), verified fixed by running the affected test classes in full isolation, not just as part of the full suite.
  • Test fixes: seeded missing FK prerequisites (model_operations.models, sell_decisions) in ApprovalWorkflowTests/TradeExecutionTests, corrected a SellPriorityRanker test input against the approved VS-10-SLICE_SPEC age-boost threshold, fixed a GDPR redaction assertion that called ToString() on a Dictionary instead of inspecting its values.

Test plan

  • dotnet build KArtSell.sln -c Release succeeds consistently (previously failed non-deterministically on stale asset filenames)
  • dotnet test unit + architecture tests: 85/85 PASS
  • dotnet test integration tests: 240/256 PASS, 4 skipped; remaining 12 failures are kartsell DB user not owning kartsell_migration_test (infra/DBA issue, unrelated to this PR - see notes below)
  • ApprovalWorkflow, Compliance, SellDecision, TradeExecution, PortfolioReconciliation test classes re-verified passing when run in isolation (--filter), not just as part of the full suite, since that's what originally hid the race condition

Known unrelated issue (not fixed here)

12 DbUpMigrationTests failures: 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. Documented in docs/CURRENT/CATALOGS/WBS_PROGRESS_TRACKER.csv (AEG-X-004 row) in the companion docs PR.

🤖 Generated with Claude Code

## Summary - `KArtSell.Host.csproj`: the FrontendFiles glob was evaluated at project-load time, before `pnpm build` ran, so `dotnet build -c Release` copied stale/missing Vite-hashed filenames on every build. Fixed by moving the glob inside the target, after the build step. - Fixed 4 real Dapper mapping bugs never caught by unit tests, all in code merged without ever running against a live database: `DateOnly` params in `ApprovalSql` (VS-03), `inet`/`jsonb` column reads in `AuditSql` (VS-04), `GdprRetention.RetentionEndsAt` typed `DateTime` against a `DATE` column, and `TradeSql.UpdateTradeStatusAsync` silently dropping `kis_order_id`/`executed_quantity`/`unit_price`/`commission`/`net_proceeds`/timestamps on every call (Phase 3 K). - Root-caused and fixed a process-wide race condition: Dapper's snake_case-to-PascalCase column mapping is set by a `[ModuleInitializer]` in `KArtSell.BuildingBlocks` that only fires once that assembly is actually loaded. `TradeSql`/`AuditSql` don't reliably touch a BuildingBlocks type at runtime, so depending on unrelated test/host startup order, every snake_case column could silently map to null. Replaced with one `[ModuleInitializer]` per module assembly (`KArtSell.Modules.ModelOperations`, `KArtSell.Modules.SignalEngine`), verified fixed by running the affected test classes in full isolation, not just as part of the full suite. - Test fixes: seeded missing FK prerequisites (`model_operations.models`, `sell_decisions`) in `ApprovalWorkflowTests`/`TradeExecutionTests`, corrected a `SellPriorityRanker` test input against the approved `VS-10-SLICE_SPEC` age-boost threshold, fixed a GDPR redaction assertion that called `ToString()` on a `Dictionary` instead of inspecting its values. ## Test plan - [x] `dotnet build KArtSell.sln -c Release` succeeds consistently (previously failed non-deterministically on stale asset filenames) - [x] `dotnet test` unit + architecture tests: 85/85 PASS - [x] `dotnet test` integration tests: 240/256 PASS, 4 skipped; remaining 12 failures are `kartsell` DB user not owning `kartsell_migration_test` (infra/DBA issue, unrelated to this PR - see notes below) - [x] `ApprovalWorkflow`, `Compliance`, `SellDecision`, `TradeExecution`, `PortfolioReconciliation` test classes re-verified passing when run **in isolation** (`--filter`), not just as part of the full suite, since that's what originally hid the race condition ## Known unrelated issue (not fixed here) 12 `DbUpMigrationTests` failures: `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. Documented in `docs/CURRENT/CATALOGS/WBS_PROGRESS_TRACKER.csv` (AEG-X-004 row) in the companion docs PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
kjh2064 added 2 commits 2026-08-07 23:46:26 +09:00
fix: Release build breakage + Dapper mapping bugs in VS-03/VS-04/Phase3-K 2ccf74c410 
- 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>
fix: Dapper underscore-mapping race condition affects AuditSql too, not just TradeSql 8ed2bcf56f 
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 merged commit 6c6011a62d into main 2026-08-07 23:47:41 +09:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
kjh2064 (김재현)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kjh2064/KArtSell.Aegis#30
Delete Branch "fix/dapper-underscore-mapping-and-build"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?