From b002a9452351a6272854e7b4ed547fc321bae4e4 Mon Sep 17 00:00:00 2001 From: Frank Tovar Date: Sun, 15 Mar 2026 12:07:50 +0100 Subject: [PATCH] Add critical cell reparse comparison review --- docs/player_gm_ux_redesign_plan.md | 14 +- .../Components/Pages/Tables.razor | 6 + .../Shared/CriticalBranchEditorModel.cs | 18 ++ .../Shared/CriticalCellEditorDialog.razor | 217 +++++++++++++++++- .../Shared/CriticalCellEditorModel.cs | 32 ++- .../Shared/CriticalEffectEditorModel.cs | 19 ++ .../Shared/CriticalResultPreviewCard.razor | 57 +++++ .../Features/CriticalCellComparisonState.cs | 9 + .../Features/CriticalCellEditorResponse.cs | 3 +- .../Features/LookupService.cs | 75 +++++- src/RolemasterDb.App/wwwroot/app.css | 19 ++ 11 files changed, 451 insertions(+), 18 deletions(-) create mode 100644 src/RolemasterDb.App/Components/Shared/CriticalResultPreviewCard.razor create mode 100644 src/RolemasterDb.App/Features/CriticalCellComparisonState.cs diff --git a/docs/player_gm_ux_redesign_plan.md b/docs/player_gm_ux_redesign_plan.md index b4e67bd..6f4e1c6 100644 --- a/docs/player_gm_ux_redesign_plan.md +++ b/docs/player_gm_ux_redesign_plan.md @@ -507,6 +507,7 @@ The following groundwork is already implemented in the web app as of March 15, 2 - the critical table browser is framed for play use rather than import administration - critical cells support re-parse from shared parser logic - advanced diagnostics have been separated from the primary editing flow +- advanced curation now includes a generated-versus-current comparison view for re-parse review These changes are real and complete, but they are no longer the active roadmap because the detailed acceptance checklist still has unfinished items. @@ -575,6 +576,17 @@ Acceptance criteria: ### Phase 4: Compare and review workflow +Status: + +- implemented in the web app on March 15, 2026 + +Implemented model: + +- re-parse now returns the freshly generated parser output alongside the merged editor state +- the editor captures the pre-reparse card as a review baseline before applying the merged state +- advanced curation shows side-by-side cards for the prior edit, the fresh generated parse, and the merged post-reparse result when available +- diff summary chips call out result-text, base-effect, and condition changes without requiring raw JSON inspection + Scope: - add a clear comparison surface for generated result versus current edited result @@ -641,4 +653,4 @@ Mitigation: ## Recommended Next Step -Implement the new Phase 3 next. The remaining structural gap is generated-versus-overridden state, so re-parse and future import refreshes can preserve intentional curation instead of treating all edited values the same. +Implement Phase 5 next. The remaining cleanup is to tighten the boundary between compact curation tools and engineering diagnostics so ordinary correction workflows stay user-facing while deeper diagnostics remain available behind an explicit advanced surface. diff --git a/src/RolemasterDb.App/Components/Pages/Tables.razor b/src/RolemasterDb.App/Components/Pages/Tables.razor index 4a53635..7650b13 100644 --- a/src/RolemasterDb.App/Components/Pages/Tables.razor +++ b/src/RolemasterDb.App/Components/Pages/Tables.razor @@ -206,6 +206,7 @@ { effect.ToItem()).ToList() }); + + public CriticalBranchEditorModel Clone() => + new() + { + BranchKind = BranchKind, + ConditionKey = ConditionKey, + ConditionText = ConditionText, + ConditionJson = ConditionJson, + RawText = RawText, + DescriptionText = DescriptionText, + RawAffixText = RawAffixText, + ParsedJson = ParsedJson, + SortOrder = SortOrder, + OriginKey = OriginKey, + IsOverridden = IsOverridden, + AreEffectsOverridden = AreEffectsOverridden, + Effects = Effects.Select(effect => effect.Clone()).ToList() + }; } diff --git a/src/RolemasterDb.App/Components/Shared/CriticalCellEditorDialog.razor b/src/RolemasterDb.App/Components/Shared/CriticalCellEditorDialog.razor index d276dff..3ec72eb 100644 --- a/src/RolemasterDb.App/Components/Shared/CriticalCellEditorDialog.razor +++ b/src/RolemasterDb.App/Components/Shared/CriticalCellEditorDialog.razor @@ -74,6 +74,10 @@ {

@GetParserNoteSummary(Model.ValidationMessages.Count)

} + @if (HasComparisonDifferences(Model, ComparisonBaseline)) + { +

Fresh parsing differs from the current edited card. Review Generated Compare before saving if you want to keep the overrides.

+ }
@@ -176,11 +180,55 @@
- Advanced Diagnostics - @GetAdvancedSummary(Model) + Advanced Review & Diagnostics + @GetAdvancedSummary(Model, ComparisonBaseline)
+ @if (Model.GeneratedState is not null) + { +
+
+
+ Generated Compare +

Compare the current edited card against the fresh parser output from the raw text.

+
+
+ +
+ @foreach (var item in GetComparisonSummaryItems(Model, ComparisonBaseline)) + { + @item + } +
+ +
+ + + @if (ComparisonBaseline is not null) + { + + } +
+
+ } +
@@ -277,6 +325,9 @@ [Parameter, EditorRequired] public CriticalCellEditorModel? Model { get; set; } + [Parameter] + public CriticalCellEditorModel? ComparisonBaseline { get; set; } + [Parameter] public bool IsLoading { get; set; } @@ -462,10 +513,23 @@ effect.IsOverridden = true; } - private static string GetAdvancedSummary(CriticalCellEditorModel model) + private static string GetAdvancedSummary(CriticalCellEditorModel model, CriticalCellEditorModel? comparisonBaseline) { + var differenceCount = GetComparisonDifferenceCount(model, comparisonBaseline); var noteCount = model.ValidationMessages.Count; - return noteCount == 0 ? "Parser metadata and save payload" : $"{noteCount} parser note{(noteCount == 1 ? string.Empty : "s")}"; + var segments = new List(); + + if (differenceCount > 0) + { + segments.Add($"{differenceCount} review change{(differenceCount == 1 ? string.Empty : "s")}"); + } + + if (noteCount > 0) + { + segments.Add($"{noteCount} parser note{(noteCount == 1 ? string.Empty : "s")}"); + } + + return segments.Count == 0 ? "Generated compare and diagnostics" : string.Join(" ยท ", segments); } private static string GetParserNoteSummary(int noteCount) => @@ -494,6 +558,135 @@ private static string BuildCurrentSavePayloadJson(CriticalCellEditorModel model) => JsonSerializer.Serialize(model.ToRequest(), DiagnosticJsonOptions); + private static IReadOnlyList GetComparisonSummaryItems(CriticalCellEditorModel model, CriticalCellEditorModel? comparisonBaseline) + { + if (model.GeneratedState is null) + { + return ["Generated comparison is unavailable."]; + } + + var items = new List(); + + if (DescriptionDiffers(model, comparisonBaseline)) + { + items.Add("Result text differs"); + } + + if (EffectsDiffer(model, comparisonBaseline)) + { + items.Add("Base effects differ"); + } + + if (BranchesDiffer(model, comparisonBaseline)) + { + items.Add("Conditions differ"); + } + + if (items.Count == 0) + { + items.Add("Current card matches the fresh parse"); + } + + if (model.GeneratedState.ValidationMessages.Count > 0) + { + items.Add($"{model.GeneratedState.ValidationMessages.Count} parser note{(model.GeneratedState.ValidationMessages.Count == 1 ? string.Empty : "s")}"); + } + + return items; + } + + private static int GetComparisonDifferenceCount(CriticalCellEditorModel model, CriticalCellEditorModel? comparisonBaseline) + { + var count = 0; + + if (DescriptionDiffers(model, comparisonBaseline)) + { + count++; + } + + if (EffectsDiffer(model, comparisonBaseline)) + { + count++; + } + + if (BranchesDiffer(model, comparisonBaseline)) + { + count++; + } + + return count; + } + + private static bool HasComparisonDifferences(CriticalCellEditorModel? model, CriticalCellEditorModel? comparisonBaseline) => + model is not null && GetComparisonDifferenceCount(model, comparisonBaseline) > 0; + + private static bool DescriptionDiffers(CriticalCellEditorModel model, CriticalCellEditorModel? comparisonBaseline) => + model.GeneratedState is not null && + !string.Equals( + NormalizeDisplayText(GetComparisonSourceModel(model, comparisonBaseline).DescriptionText), + NormalizeDisplayText(model.GeneratedState.DescriptionText), + StringComparison.Ordinal); + + private static bool EffectsDiffer(CriticalCellEditorModel model, CriticalCellEditorModel? comparisonBaseline) => + model.GeneratedState is not null && + SerializeComparisonValue(BuildPreviewEffects(GetComparisonSourceModel(model, comparisonBaseline)).Select(ProjectEffectForComparison).ToList()) != + SerializeComparisonValue(model.GeneratedState.Effects.Select(ProjectEffectForComparison).ToList()); + + private static bool BranchesDiffer(CriticalCellEditorModel model, CriticalCellEditorModel? comparisonBaseline) => + model.GeneratedState is not null && + SerializeComparisonValue(BuildPreviewBranches(GetComparisonSourceModel(model, comparisonBaseline)).Select(ProjectBranchForComparison).ToList()) != + SerializeComparisonValue(model.GeneratedState.Branches.Select(ProjectBranchForComparison).ToList()); + + private static string NormalizeDisplayText(string? value) => + value?.Trim() ?? string.Empty; + + private static string SerializeComparisonValue(TValue value) => + JsonSerializer.Serialize(value, DiagnosticJsonOptions); + + private static object ProjectEffectForComparison(CriticalEffectLookupResponse effect) => new + { + effect.EffectCode, + effect.Target, + effect.ValueInteger, + effect.ValueExpression, + effect.DurationRounds, + effect.PerRound, + effect.Modifier, + effect.BodyPart, + effect.IsPermanent, + SourceText = NormalizeDisplayText(effect.SourceText) + }; + + private static object ProjectBranchForComparison(CriticalBranchLookupResponse branch) => new + { + Condition = NormalizeDisplayText(branch.ConditionText), + Description = NormalizeDisplayText(branch.Description), + Effects = branch.Effects + .Select(ProjectEffectForComparison) + .ToList() + }; + + private static CriticalCellEditorModel GetComparisonSourceModel( + CriticalCellEditorModel model, + CriticalCellEditorModel? comparisonBaseline) => + comparisonBaseline ?? model; + + private static string GetCurrentComparisonCaption(CriticalCellEditorModel? comparisonBaseline) => + comparisonBaseline is null + ? "This is the result that will be saved." + : "This is the edited card before the last re-parse."; + + private static IReadOnlyList BuildPreviewEffects(CriticalCellEditorModel model) => + model.Effects + .Select(CreatePreviewEffect) + .ToList(); + + private static IReadOnlyList BuildPreviewBranches(CriticalCellEditorModel model) => + model.Branches + .OrderBy(branch => branch.SortOrder) + .Select(CreatePreviewBranch) + .ToList(); + private static List<(string Scope, string EffectLabel, string SourceType, string? SourceText)> GetEffectMetadataRows(CriticalCellEditorModel model) { var rows = new List<(string Scope, string EffectLabel, string SourceType, string? SourceText)>(); @@ -541,6 +734,22 @@ effect.IsPermanent, effect.SourceType, effect.SourceText); + + private static CriticalEffectLookupResponse CreatePreviewEffect(CriticalEffectEditorModel effect) => + CreateBadgeEffect(effect); + + private static CriticalBranchLookupResponse CreatePreviewBranch(CriticalBranchEditorModel branch) => + new( + branch.BranchKind, + branch.ConditionKey, + branch.ConditionText, + branch.DescriptionText, + branch.RawAffixText, + branch.Effects + .Select(CreatePreviewEffect) + .ToList(), + branch.RawText, + branch.SortOrder); } @functions { diff --git a/src/RolemasterDb.App/Components/Shared/CriticalCellEditorModel.cs b/src/RolemasterDb.App/Components/Shared/CriticalCellEditorModel.cs index 2f895a4..d908048 100644 --- a/src/RolemasterDb.App/Components/Shared/CriticalCellEditorModel.cs +++ b/src/RolemasterDb.App/Components/Shared/CriticalCellEditorModel.cs @@ -27,6 +27,7 @@ public sealed class CriticalCellEditorModel public List ValidationMessages { get; set; } = []; public List Effects { get; set; } = []; public List Branches { get; set; } = []; + public CriticalCellComparisonState? GeneratedState { get; set; } public static CriticalCellEditorModel FromResponse(CriticalCellEditorResponse response) => new() @@ -52,7 +53,8 @@ public sealed class CriticalCellEditorModel AreBranchesOverridden = response.AreBranchesOverridden, ValidationMessages = response.ValidationMessages.ToList(), Effects = response.Effects.Select(CriticalEffectEditorModel.FromItem).ToList(), - Branches = response.Branches.Select(CriticalBranchEditorModel.FromItem).ToList() + Branches = response.Branches.Select(CriticalBranchEditorModel.FromItem).ToList(), + GeneratedState = response.GeneratedState }; public CriticalCellUpdateRequest ToRequest() @@ -83,6 +85,34 @@ public sealed class CriticalCellEditorModel }; } + public CriticalCellEditorModel Clone() => + new() + { + ResultId = ResultId, + TableSlug = TableSlug, + TableName = TableName, + SourceDocument = SourceDocument, + RollBand = RollBand, + GroupKey = GroupKey, + GroupLabel = GroupLabel, + ColumnKey = ColumnKey, + ColumnLabel = ColumnLabel, + ColumnRole = ColumnRole, + RawCellText = RawCellText, + DescriptionText = DescriptionText, + RawAffixText = RawAffixText, + ParseStatus = ParseStatus, + ParsedJson = ParsedJson, + IsDescriptionOverridden = IsDescriptionOverridden, + IsRawAffixTextOverridden = IsRawAffixTextOverridden, + AreEffectsOverridden = AreEffectsOverridden, + AreBranchesOverridden = AreBranchesOverridden, + ValidationMessages = ValidationMessages.ToList(), + Effects = Effects.Select(effect => effect.Clone()).ToList(), + Branches = Branches.Select(branch => branch.Clone()).ToList(), + GeneratedState = GeneratedState + }; + private static string ResolveParseStatus( IReadOnlyList effects, IReadOnlyList branches) => diff --git a/src/RolemasterDb.App/Components/Shared/CriticalEffectEditorModel.cs b/src/RolemasterDb.App/Components/Shared/CriticalEffectEditorModel.cs index ebc0396..223da85 100644 --- a/src/RolemasterDb.App/Components/Shared/CriticalEffectEditorModel.cs +++ b/src/RolemasterDb.App/Components/Shared/CriticalEffectEditorModel.cs @@ -54,4 +54,23 @@ public sealed class CriticalEffectEditorModel SourceText, OriginKey, IsOverridden); + + public CriticalEffectEditorModel Clone() => + new() + { + EffectCode = EffectCode, + Target = Target, + ValueInteger = ValueInteger, + ValueDecimal = ValueDecimal, + ValueExpression = ValueExpression, + DurationRounds = DurationRounds, + PerRound = PerRound, + Modifier = Modifier, + BodyPart = BodyPart, + IsPermanent = IsPermanent, + SourceType = SourceType, + SourceText = SourceText, + OriginKey = OriginKey, + IsOverridden = IsOverridden + }; } diff --git a/src/RolemasterDb.App/Components/Shared/CriticalResultPreviewCard.razor b/src/RolemasterDb.App/Components/Shared/CriticalResultPreviewCard.razor new file mode 100644 index 0000000..757e1a1 --- /dev/null +++ b/src/RolemasterDb.App/Components/Shared/CriticalResultPreviewCard.razor @@ -0,0 +1,57 @@ +@using System +@using System.Collections.Generic +@using RolemasterDb.App.Features + +
+
+
+ @Title + @if (!string.IsNullOrWhiteSpace(Caption)) + { +

@Caption

+ } +
+
+ + @if ((Effects?.Count ?? 0) == 0 && (Branches?.Count ?? 0) == 0 && string.IsNullOrWhiteSpace(Description)) + { +

No visible result is available for this card.

+ } + else + { + + } + + @if ((Notes?.Count ?? 0) > 0) + { +
+ @foreach (var note in Notes!) + { +

@note

+ } +
+ } +
+ +@code { + [Parameter, EditorRequired] + public string Title { get; set; } = string.Empty; + + [Parameter] + public string? Caption { get; set; } + + [Parameter, EditorRequired] + public string Description { get; set; } = string.Empty; + + [Parameter] + public IReadOnlyList? Effects { get; set; } + + [Parameter] + public IReadOnlyList? Branches { get; set; } + + [Parameter] + public IReadOnlyList? Notes { get; set; } +} diff --git a/src/RolemasterDb.App/Features/CriticalCellComparisonState.cs b/src/RolemasterDb.App/Features/CriticalCellComparisonState.cs new file mode 100644 index 0000000..7b00188 --- /dev/null +++ b/src/RolemasterDb.App/Features/CriticalCellComparisonState.cs @@ -0,0 +1,9 @@ +using System.Collections.Generic; + +namespace RolemasterDb.App.Features; + +public sealed record CriticalCellComparisonState( + string DescriptionText, + IReadOnlyList Effects, + IReadOnlyList Branches, + IReadOnlyList ValidationMessages); diff --git a/src/RolemasterDb.App/Features/CriticalCellEditorResponse.cs b/src/RolemasterDb.App/Features/CriticalCellEditorResponse.cs index ff893c6..f162e39 100644 --- a/src/RolemasterDb.App/Features/CriticalCellEditorResponse.cs +++ b/src/RolemasterDb.App/Features/CriticalCellEditorResponse.cs @@ -24,4 +24,5 @@ public sealed record CriticalCellEditorResponse( bool AreBranchesOverridden, IReadOnlyList ValidationMessages, IReadOnlyList Effects, - IReadOnlyList Branches); + IReadOnlyList Branches, + CriticalCellComparisonState? GeneratedState); diff --git a/src/RolemasterDb.App/Features/LookupService.cs b/src/RolemasterDb.App/Features/LookupService.cs index 72d1c57..49a4066 100644 --- a/src/RolemasterDb.App/Features/LookupService.cs +++ b/src/RolemasterDb.App/Features/LookupService.cs @@ -290,7 +290,14 @@ public sealed class LookupService(IDbContextFactory dbConte item => item.Id == resultId && item.CriticalTable.Slug == normalizedSlug, cancellationToken); - return result is null ? null : CreateCellEditorResponse(result); + if (result is null) + { + return null; + } + + var currentState = CreateCurrentEditorState(result); + var generatedContent = await ParseCriticalCellContentAsync(dbContext, result.CriticalTableId, currentState.RawCellText, cancellationToken); + return CreateCellEditorResponse(result, currentState, generatedContent.ValidationErrors, CreateComparisonState(generatedContent)); } public async Task ReparseCriticalCellAsync( @@ -322,7 +329,7 @@ public sealed class LookupService(IDbContextFactory dbConte var content = SharedParsing.CriticalCellTextParser.Parse(currentState.RawCellText, affixLegend); var generatedState = CreateGeneratedEditorState(content); var mergedState = MergeGeneratedState(currentState, generatedState); - return CreateCellEditorResponse(result, mergedState, content.ValidationErrors); + return CreateCellEditorResponse(result, mergedState, content.ValidationErrors, CreateComparisonState(content)); } public async Task UpdateCriticalCellAsync( @@ -363,7 +370,8 @@ public sealed class LookupService(IDbContextFactory dbConte await dbContext.SaveChangesAsync(cancellationToken); - return CreateCellEditorResponse(result, request, []); + var generatedContent = await ParseCriticalCellContentAsync(dbContext, result.CriticalTableId, request.RawCellText, cancellationToken); + return CreateCellEditorResponse(result, request, generatedContent.ValidationErrors, CreateComparisonState(generatedContent)); } private static IReadOnlyList BuildLegend(IReadOnlyList cells) @@ -424,16 +432,11 @@ public sealed class LookupService(IDbContextFactory dbConte effect.SourceType, effect.SourceText); - private static CriticalCellEditorResponse CreateCellEditorResponse(CriticalResult result) - { - var state = CreateCurrentEditorState(result); - return CreateCellEditorResponse(result, state, []); - } - private static CriticalCellEditorResponse CreateCellEditorResponse( CriticalResult result, CriticalCellUpdateRequest state, - IReadOnlyList validationMessages) + IReadOnlyList validationMessages, + CriticalCellComparisonState? generatedState) { var snapshotJson = CriticalCellEditorSnapshot.FromRequest(state).ToJson(); @@ -459,7 +462,8 @@ public sealed class LookupService(IDbContextFactory dbConte state.AreBranchesOverridden, validationMessages.ToList(), state.Effects.ToList(), - state.Branches.ToList()); + state.Branches.ToList(), + generatedState); } private static CriticalBranchLookupResponse CreateBranchLookupResponse(CriticalBranch branch) => @@ -521,6 +525,18 @@ public sealed class LookupService(IDbContextFactory dbConte .ToList()); } + private static CriticalCellComparisonState CreateComparisonState(SharedParsing.CriticalCellParseContent content) => + new( + content.DescriptionText, + content.Effects + .Select(CreateEffectLookupResponse) + .ToList(), + content.Branches + .OrderBy(branch => branch.SortOrder) + .Select(CreateBranchLookupResponse) + .ToList(), + content.ValidationErrors.ToList()); + private static CriticalEffectEditorItem CreateEffectEditorItem(CriticalEffect effect, string originKey) => new( effect.EffectCode, @@ -806,6 +822,33 @@ public sealed class LookupService(IDbContextFactory dbConte SourceText = NormalizeOptionalText(effect.SourceText) }; + private static CriticalEffectLookupResponse CreateEffectLookupResponse(SharedParsing.ParsedCriticalEffect effect) => + new( + effect.EffectCode, + effect.Target, + effect.ValueInteger, + effect.ValueExpression, + effect.DurationRounds, + effect.PerRound, + effect.Modifier, + effect.BodyPart, + effect.IsPermanent, + effect.SourceType, + effect.SourceText); + + private static CriticalBranchLookupResponse CreateBranchLookupResponse(SharedParsing.ParsedCriticalBranch branch) => + new( + branch.BranchKind, + branch.ConditionKey, + branch.ConditionText, + branch.DescriptionText, + branch.RawAffixText, + branch.Effects + .Select(CreateEffectLookupResponse) + .ToList(), + branch.RawText, + branch.SortOrder); + private static string ResolveParseStatus( IReadOnlyList effects, IReadOnlyList branches) => @@ -876,6 +919,16 @@ public sealed class LookupService(IDbContextFactory dbConte supportsPowerPointModifier); } + private static async Task ParseCriticalCellContentAsync( + RolemasterDbContext dbContext, + int tableId, + string rawCellText, + CancellationToken cancellationToken) + { + var affixLegend = await BuildSharedAffixLegendAsync(dbContext, tableId, cancellationToken); + return SharedParsing.CriticalCellTextParser.Parse(rawCellText, affixLegend); + } + private static bool IsLegendSymbolEffectCode(string effectCode) => effectCode is CriticalEffectCodes.MustParryRounds or CriticalEffectCodes.NoParryRounds diff --git a/src/RolemasterDb.App/wwwroot/app.css b/src/RolemasterDb.App/wwwroot/app.css index 5e2bfb5..461cf26 100644 --- a/src/RolemasterDb.App/wwwroot/app.css +++ b/src/RolemasterDb.App/wwwroot/app.css @@ -834,6 +834,25 @@ textarea { margin: 0; } +.critical-editor-compare-grid { + display: grid; + gap: 0.85rem; + grid-template-columns: repeat(auto-fit, minmax(240px, 1fr)); +} + +.critical-editor-compare-card { + display: grid; + gap: 0.75rem; + padding: 0.9rem; + border-radius: 16px; + background: rgba(255, 255, 255, 0.72); + border: 1px solid rgba(127, 96, 55, 0.12); +} + +.critical-editor-compare-card .critical-cell { + min-height: 100%; +} + .critical-editor-inline-list { display: grid; gap: 0.55rem;