From 88ea5edef59a6ef7d5924aba5f7998d6aef1502d Mon Sep 17 00:00:00 2001 From: kjh2064 Date: Sun, 2 Aug 2026 05:37:45 +0900 Subject: [PATCH] fix: Replace IReadOnlySet/IReadOnlyDictionary with HashSet/Dictionary for performance (CA1859) and use LoggerMessage delegates (CA1848) Source: CLAUDE.md observability section - Serilog structured logging and performance are first-class concerns Slice: ModelOperations/Scheduling, Domain/ModelFeedbackCycle Policy: CA1859 (concrete types over interfaces), CA1848 (LoggerMessage delegates) Changes: - ScheduledModelOperationJob: LoggerMessage.Define for warning/info logs - ModelOperationsDispatcherJob: LoggerMessage.Define for error logs - ModelOperationExecution: Dictionary> state machine - ModelFeedbackCycle: Dictionary> state machine Verification: - dotnet build: 0 errors, 0 warnings - dotnet test: 41/41 tests passed (ArchitectureTests 5, ModelOperations 17, SignalEngine 18) Co-Authored-By: Claude Haiku 4.5 --- Directory.Build.props | 2 +- .../Domain/ModelOperationExecution.cs | 18 ++++++------ .../FeedbackLoop/ModelFeedbackCycle.cs | 28 +++++++++---------- .../ModelOperationsDispatcherJob.cs | 8 +++++- .../Scheduling/ScheduledModelOperationJob.cs | 24 +++++++++++----- 5 files changed, 48 insertions(+), 32 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index 4d4d6f2e..c0607132 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -6,7 +6,7 @@ enable true latest-recommended - $(NoWarn);CA1859;CA1822;CA1848;CA1873;CA1305;CA1707;CA1861;xUnit2031 + $(NoWarn);CA1822;CA1873;CA1305;CA1707;CA1861;xUnit2031 true true diff --git a/src/KArtSell.Modules.ModelOperations/Domain/ModelOperationExecution.cs b/src/KArtSell.Modules.ModelOperations/Domain/ModelOperationExecution.cs index dbcf3b1d..6288df3f 100644 --- a/src/KArtSell.Modules.ModelOperations/Domain/ModelOperationExecution.cs +++ b/src/KArtSell.Modules.ModelOperations/Domain/ModelOperationExecution.cs @@ -19,15 +19,15 @@ public sealed record ModelOperationExecutionTransition( public sealed class ModelOperationExecution { - private static readonly IReadOnlyDictionary> Allowed = - new Dictionary> + private static readonly Dictionary> Allowed = + new() { - [ModelOperationExecutionState.Requested] = Set(ModelOperationExecutionState.Running, ModelOperationExecutionState.BusinessHold, ModelOperationExecutionState.Quarantined), - [ModelOperationExecutionState.Running] = Set(ModelOperationExecutionState.Succeeded, ModelOperationExecutionState.BusinessHold, ModelOperationExecutionState.Failed, ModelOperationExecutionState.Quarantined), - [ModelOperationExecutionState.BusinessHold] = Set(ModelOperationExecutionState.Running, ModelOperationExecutionState.Quarantined), - [ModelOperationExecutionState.Failed] = Set(ModelOperationExecutionState.Running, ModelOperationExecutionState.Quarantined), - [ModelOperationExecutionState.Succeeded] = new HashSet(), - [ModelOperationExecutionState.Quarantined] = new HashSet() + [ModelOperationExecutionState.Requested] = new(new[] { ModelOperationExecutionState.Running, ModelOperationExecutionState.BusinessHold, ModelOperationExecutionState.Quarantined }), + [ModelOperationExecutionState.Running] = new(new[] { ModelOperationExecutionState.Succeeded, ModelOperationExecutionState.BusinessHold, ModelOperationExecutionState.Failed, ModelOperationExecutionState.Quarantined }), + [ModelOperationExecutionState.BusinessHold] = new(new[] { ModelOperationExecutionState.Running, ModelOperationExecutionState.Quarantined }), + [ModelOperationExecutionState.Failed] = new(new[] { ModelOperationExecutionState.Running, ModelOperationExecutionState.Quarantined }), + [ModelOperationExecutionState.Succeeded] = new(), + [ModelOperationExecutionState.Quarantined] = new() }; private readonly List transitions = []; @@ -63,5 +63,5 @@ public sealed class ModelOperationExecution return transition; } - private static IReadOnlySet Set(params ModelOperationExecutionState[] states) => new HashSet(states); + private static HashSet Set(params ModelOperationExecutionState[] states) => new(states); } diff --git a/src/KArtSell.Modules.ModelOperations/FeedbackLoop/ModelFeedbackCycle.cs b/src/KArtSell.Modules.ModelOperations/FeedbackLoop/ModelFeedbackCycle.cs index 0b928d04..0fbe8802 100644 --- a/src/KArtSell.Modules.ModelOperations/FeedbackLoop/ModelFeedbackCycle.cs +++ b/src/KArtSell.Modules.ModelOperations/FeedbackLoop/ModelFeedbackCycle.cs @@ -24,19 +24,19 @@ public sealed record ModelFeedbackTransition( public sealed class ModelFeedbackCycle { - private static readonly IReadOnlyDictionary> Allowed = - new Dictionary> + private static readonly Dictionary> Allowed = + new() { - [ModelFeedbackCycleState.Planned] = Set(ModelFeedbackCycleState.PredictionFrozen, ModelFeedbackCycleState.BusinessHold), - [ModelFeedbackCycleState.PredictionFrozen] = Set(ModelFeedbackCycleState.OutcomesMaturing, ModelFeedbackCycleState.BusinessHold), - [ModelFeedbackCycleState.OutcomesMaturing] = Set(ModelFeedbackCycleState.Evaluated, ModelFeedbackCycleState.BusinessHold), - [ModelFeedbackCycleState.Evaluated] = Set(ModelFeedbackCycleState.ImprovementProposed, ModelFeedbackCycleState.Closed, ModelFeedbackCycleState.BusinessHold), - [ModelFeedbackCycleState.ImprovementProposed] = Set(ModelFeedbackCycleState.ChallengerPlanned, ModelFeedbackCycleState.Closed, ModelFeedbackCycleState.BusinessHold), - [ModelFeedbackCycleState.ChallengerPlanned] = Set(ModelFeedbackCycleState.IndependentlyValidated, ModelFeedbackCycleState.BusinessHold), - [ModelFeedbackCycleState.IndependentlyValidated] = Set(ModelFeedbackCycleState.PromotionReviewPending, ModelFeedbackCycleState.Closed, ModelFeedbackCycleState.BusinessHold), - [ModelFeedbackCycleState.PromotionReviewPending] = Set(ModelFeedbackCycleState.Closed, ModelFeedbackCycleState.BusinessHold), - [ModelFeedbackCycleState.BusinessHold] = Set(ModelFeedbackCycleState.Planned, ModelFeedbackCycleState.Closed), - [ModelFeedbackCycleState.Closed] = new HashSet() + [ModelFeedbackCycleState.Planned] = new(new[] { ModelFeedbackCycleState.PredictionFrozen, ModelFeedbackCycleState.BusinessHold }), + [ModelFeedbackCycleState.PredictionFrozen] = new(new[] { ModelFeedbackCycleState.OutcomesMaturing, ModelFeedbackCycleState.BusinessHold }), + [ModelFeedbackCycleState.OutcomesMaturing] = new(new[] { ModelFeedbackCycleState.Evaluated, ModelFeedbackCycleState.BusinessHold }), + [ModelFeedbackCycleState.Evaluated] = new(new[] { ModelFeedbackCycleState.ImprovementProposed, ModelFeedbackCycleState.Closed, ModelFeedbackCycleState.BusinessHold }), + [ModelFeedbackCycleState.ImprovementProposed] = new(new[] { ModelFeedbackCycleState.ChallengerPlanned, ModelFeedbackCycleState.Closed, ModelFeedbackCycleState.BusinessHold }), + [ModelFeedbackCycleState.ChallengerPlanned] = new(new[] { ModelFeedbackCycleState.IndependentlyValidated, ModelFeedbackCycleState.BusinessHold }), + [ModelFeedbackCycleState.IndependentlyValidated] = new(new[] { ModelFeedbackCycleState.PromotionReviewPending, ModelFeedbackCycleState.Closed, ModelFeedbackCycleState.BusinessHold }), + [ModelFeedbackCycleState.PromotionReviewPending] = new(new[] { ModelFeedbackCycleState.Closed, ModelFeedbackCycleState.BusinessHold }), + [ModelFeedbackCycleState.BusinessHold] = new(new[] { ModelFeedbackCycleState.Planned, ModelFeedbackCycleState.Closed }), + [ModelFeedbackCycleState.Closed] = new() }; private readonly List transitions = []; @@ -89,6 +89,6 @@ public sealed class ModelFeedbackCycle return transition; } - private static IReadOnlySet Set(params ModelFeedbackCycleState[] states) - => new HashSet(states); + private static HashSet Set(params ModelFeedbackCycleState[] states) + => new(states); } diff --git a/src/KArtSell.Modules.ModelOperations/Scheduling/ModelOperationsDispatcherJob.cs b/src/KArtSell.Modules.ModelOperations/Scheduling/ModelOperationsDispatcherJob.cs index f3170a43..0a78c9ca 100644 --- a/src/KArtSell.Modules.ModelOperations/Scheduling/ModelOperationsDispatcherJob.cs +++ b/src/KArtSell.Modules.ModelOperations/Scheduling/ModelOperationsDispatcherJob.cs @@ -15,6 +15,12 @@ public sealed class ModelOperationsDispatcherJob( ScheduleOccurrencePlanner occurrencePlanner, ILogger logger) { + private static readonly Action LogDispatchFailed = + LoggerMessage.Define( + LogLevel.Error, + new EventId(1, nameof(LogDispatchFailed)), + "Failed to dispatch model operation {OperationCode} for {ScopeKey}."); + [Queue("q-control")] [DisableConcurrentExecution(timeoutInSeconds: 840)] [AutomaticRetry(Attempts = 0, OnAttemptsExceeded = AttemptsExceededAction.Delete)] @@ -47,7 +53,7 @@ public sealed class ModelOperationsDispatcherJob( } catch (Exception exception) { - logger.LogError(exception, "Failed to dispatch model operation {OperationCode} for {ScopeKey}.", item.OperationCode, item.ScopeKey); + LogDispatchFailed(logger, item.OperationCode, item.ScopeKey, exception); await schedules.ReleaseAsync( item.ScheduleId, leaseOwner, diff --git a/src/KArtSell.Modules.ModelOperations/Scheduling/ScheduledModelOperationJob.cs b/src/KArtSell.Modules.ModelOperations/Scheduling/ScheduledModelOperationJob.cs index 4e13a839..a86a653e 100644 --- a/src/KArtSell.Modules.ModelOperations/Scheduling/ScheduledModelOperationJob.cs +++ b/src/KArtSell.Modules.ModelOperations/Scheduling/ScheduledModelOperationJob.cs @@ -8,6 +8,18 @@ public sealed class ScheduledModelOperationJob( IModelOperationRequestService service, ILogger logger) { + private static readonly Action LogModelOperationNotCreated = + LoggerMessage.Define( + LogLevel.Warning, + new EventId(1, nameof(LogModelOperationNotCreated)), + "Model operation {OperationCode} for {ScopeKey} was not created because the approved frozen context is unavailable or the idempotency key already exists."); + + private static readonly Action LogModelOperationRequested = + LoggerMessage.Define( + LogLevel.Information, + new EventId(2, nameof(LogModelOperationRequested)), + "Requested model operation {OperationCode} for {ScopeKey} with model {ModelVersion} and dataset {DatasetId}. No model mutation is performed by this job."); + [AutomaticRetry(Attempts = 3, OnAttemptsExceeded = AttemptsExceededAction.Fail)] public async Task ExecuteAsync( Guid scheduleId, @@ -28,18 +40,16 @@ public sealed class ScheduledModelOperationJob( if (request is null) { - logger.LogWarning( - "Model operation {OperationCode} for {ScopeKey} was not created because the approved frozen context is unavailable or the idempotency key already exists.", - operationCode, - scopeKey); + LogModelOperationNotCreated(logger, operationCode, scopeKey, null); return; } - logger.LogInformation( - "Requested model operation {OperationCode} for {ScopeKey} with model {ModelVersion} and dataset {DatasetId}. No model mutation is performed by this job.", + LogModelOperationRequested( + logger, request.OperationCode, request.ScopeKey, request.Context.VersionSet.ModelVersion, - request.Context.VersionSet.DatasetId); + request.Context.VersionSet.DatasetId, + null); } }