From 5e89d581572ef7322e119998a91b1524ca8d25d9 Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Fri, 24 Jul 2026 11:38:52 -0400 Subject: [PATCH 1/3] feat(engine): help system with keyword/semantic search fallback Slice 5 of the WheelMUD reconciliation roadmap. `help ` now resolves a persisted HelpTopic via a three-tier pipeline (exact name/alias, then keyword, then a threshold-gated semantic-search fallback over embedded content) instead of only listing commands. Content is authored in-game only (helptopic/helpindex, both MinorBuilder-gated) - no file-based content path, matching the existing OLC precedent rather than WheelMUD's file-based HelpManager. IEmbeddingProvider/IHelpSearchIndex are the swap seams ADR-0010 exists to protect: the shipped default (StubEmbeddingProvider, deterministic hashed-bag-of-words) needs no external dependency, while the sample app now also demonstrates a real local model (SmartComponents. LocalEmbeddings via ADR-0011) wired in behind the same interfaces with no code changes elsewhere. Building and manually exercising the real model surfaced that a fixed relevance threshold doesn't transfer between providers - moved RelevanceThreshold onto IEmbeddingProvider itself so each provider calibrates its own cutoff. Also replaces the project icon (root icon.png, used as the NuGet PackageIcon, and the docsite's nav logo/favicon) and adds an ASCII banner to the README and docsite home page. See docs/adr/0010-help-system-semantic-search.md, docs/adr/0011-local-embedding-provider-for-sample.md, and docs/plans/0010-help-system-semantic-search.md for the full design record and verification trail. --- Directory.Packages.props | 5 + README.md | 18 + docs/README.md | 1 + docs/adr/0010-help-system-semantic-search.md | 317 ++++++++++++++++++ ...011-local-embedding-provider-for-sample.md | 189 +++++++++++ docs/adr/README.md | 2 + docs/commands.md | 9 +- docs/help-system.md | 166 +++++++++ .../0001-wheelmud-reconciliation-roadmap.md | 5 +- .../plans/0010-help-system-semantic-search.md | 193 +++++++++++ docs/plans/README.md | 1 + docs/research/wheelmud-findings.md | 49 +++ docsite/docs/customizing.md | 11 +- docsite/docs/help-system.md | 153 +++++++++ docsite/docs/images/icon.png | Bin 85222 -> 322829 bytes docsite/docs/index.md | 9 + docsite/zensical.toml | 3 +- icon.png | Bin 85222 -> 322829 bytes .../LocalEmbeddingProvider.cs | 49 +++ samples/SharpMud.Samples.Classic/Program.cs | 16 +- .../SharpMud.Samples.Classic.csproj | 3 + .../Builtin/Admin/HelpAdminCommands.cs | 24 ++ .../Builtin/Admin/HelpIndexRebuildCommand.cs | 59 ++++ .../Builtin/Admin/HelpTopicEditCommand.cs | 52 +++ .../Commands/Builtin/BuiltinCommands.cs | 5 +- .../Commands/Builtin/HelpCommand.cs | 68 +++- .../Help/CosineHelpSearchIndex.cs | 65 ++++ .../Help/HelpContentHashing.cs | 13 + src/SharpMud.Engine/Help/HelpSearchHit.cs | 6 + src/SharpMud.Engine/Help/HelpTopic.cs | 69 ++++ src/SharpMud.Engine/Help/HelpTopicChunk.cs | 23 ++ src/SharpMud.Engine/Help/HelpTopicChunker.cs | 22 ++ src/SharpMud.Engine/Help/HelpTopicId.cs | 11 + .../Help/IEmbeddingProvider.cs | 32 ++ src/SharpMud.Engine/Help/IHelpRepository.cs | 25 ++ src/SharpMud.Engine/Help/IHelpSearchIndex.cs | 18 + .../Help/StubEmbeddingProvider.cs | 63 ++++ .../ServiceCollectionExtensions.cs | 3 +- .../ServiceCollectionExtensions.cs | 15 +- .../HelpTopicChunkConfiguration.cs | 38 +++ .../Configurations/HelpTopicConfiguration.cs | 49 +++ src/SharpMud.Persistence/GameDbContext.cs | 8 + src/SharpMud.Persistence/HelpRepository.cs | 77 +++++ .../Admin/HelpIndexRebuildCommandTests.cs | 57 ++++ .../Admin/HelpTopicEditCommandTests.cs | 69 ++++ .../Commands/HelpCommandTests.cs | 95 +++++- .../Help/CosineHelpSearchIndexTests.cs | 89 +++++ .../Help/HelpTopicChunkerTests.cs | 29 ++ .../Help/HelpTopicTests.cs | 60 ++++ .../Help/StubEmbeddingProviderTests.cs | 52 +++ .../HelpRepositoryTests.cs | 117 +++++++ .../ServiceCollectionExtensionsTests.cs | 5 + 52 files changed, 2495 insertions(+), 22 deletions(-) create mode 100644 docs/adr/0010-help-system-semantic-search.md create mode 100644 docs/adr/0011-local-embedding-provider-for-sample.md create mode 100644 docs/help-system.md create mode 100644 docs/plans/0010-help-system-semantic-search.md create mode 100644 docsite/docs/help-system.md create mode 100644 samples/SharpMud.Samples.Classic/LocalEmbeddingProvider.cs create mode 100644 src/SharpMud.Engine/Commands/Builtin/Admin/HelpAdminCommands.cs create mode 100644 src/SharpMud.Engine/Commands/Builtin/Admin/HelpIndexRebuildCommand.cs create mode 100644 src/SharpMud.Engine/Commands/Builtin/Admin/HelpTopicEditCommand.cs create mode 100644 src/SharpMud.Engine/Help/CosineHelpSearchIndex.cs create mode 100644 src/SharpMud.Engine/Help/HelpContentHashing.cs create mode 100644 src/SharpMud.Engine/Help/HelpSearchHit.cs create mode 100644 src/SharpMud.Engine/Help/HelpTopic.cs create mode 100644 src/SharpMud.Engine/Help/HelpTopicChunk.cs create mode 100644 src/SharpMud.Engine/Help/HelpTopicChunker.cs create mode 100644 src/SharpMud.Engine/Help/HelpTopicId.cs create mode 100644 src/SharpMud.Engine/Help/IEmbeddingProvider.cs create mode 100644 src/SharpMud.Engine/Help/IHelpRepository.cs create mode 100644 src/SharpMud.Engine/Help/IHelpSearchIndex.cs create mode 100644 src/SharpMud.Engine/Help/StubEmbeddingProvider.cs create mode 100644 src/SharpMud.Persistence/Configurations/HelpTopicChunkConfiguration.cs create mode 100644 src/SharpMud.Persistence/Configurations/HelpTopicConfiguration.cs create mode 100644 src/SharpMud.Persistence/HelpRepository.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/HelpIndexRebuildCommandTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/HelpTopicEditCommandTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Help/CosineHelpSearchIndexTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Help/HelpTopicChunkerTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Help/HelpTopicTests.cs create mode 100644 tests/SharpMud.Engine.Tests/Help/StubEmbeddingProviderTests.cs create mode 100644 tests/SharpMud.Persistence.Tests/HelpRepositoryTests.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index fefa37a..ebee2ea 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -25,6 +25,11 @@ + + + diff --git a/src/SharpMud.Engine/Commands/Builtin/Admin/HelpAdminCommands.cs b/src/SharpMud.Engine/Commands/Builtin/Admin/HelpAdminCommands.cs new file mode 100644 index 0000000..d65980c --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Admin/HelpAdminCommands.cs @@ -0,0 +1,24 @@ +using SharpMud.Engine.Help; + +namespace SharpMud.Engine.Commands.Builtin.Admin; + +/// +/// Registers the help-authoring command set (ADR-0010) - helptopic/ +/// helpindex, both at (same +/// tier as world-building/OLC - authoring content is the same class of +/// privilege). Not called automatically by - a consumer calls this themselves, +/// the same opt-in shape / +/// already use. help +/// itself (topic lookup) is always registered by +/// - only authoring is opt-in. +/// +public static class HelpAdminCommands +{ + /// Registers helptopic/helpindex against . + public static void RegisterAll(ICommandRegistry registry, IHelpRepository repository, IEmbeddingProvider embeddingProvider) + { + registry.RegisterWithRole(new HelpTopicEditCommand(repository), SecurityRole.MinorBuilder); + registry.RegisterWithRole(new HelpIndexRebuildCommand(repository, embeddingProvider), SecurityRole.MinorBuilder); + } +} diff --git a/src/SharpMud.Engine/Commands/Builtin/Admin/HelpIndexRebuildCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Admin/HelpIndexRebuildCommand.cs new file mode 100644 index 0000000..66beb24 --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Admin/HelpIndexRebuildCommand.cs @@ -0,0 +1,59 @@ +using SharpMud.Engine.Help; + +namespace SharpMud.Engine.Commands.Builtin.Admin; + +/// +/// The helpindex rebuild command (, +/// ADR-0010) - re-chunks and re-embeds every 's via , replacing +/// each topic's wholesale. No automatic +/// trigger exists (not on save, not at boot) - this command is the only way +/// the semantic-search index changes, deliberately keeping embedding-provider +/// calls out of the content-edit path (ADR-0010's Decision Outcome). +/// +public sealed class HelpIndexRebuildCommand : ICommand +{ + private readonly IHelpRepository _repository; + private readonly IEmbeddingProvider _embeddingProvider; + + /// Creates the command, rebuilding topics from via . + public HelpIndexRebuildCommand(IHelpRepository repository, IEmbeddingProvider embeddingProvider) + { + _repository = repository; + _embeddingProvider = embeddingProvider; + } + + /// + public string Verb => "helpindex"; + + /// + public IReadOnlyList Aliases { get; } = []; + + /// + public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + if (ctx.Args is not ["rebuild"]) + { + await ctx.Session.WriteLineAsync("Usage: helpindex rebuild", ct); + return; + } + + var topics = await _repository.GetAllTopicsAsync(ct); + foreach (var topic in topics) + { + var chunkTexts = HelpTopicChunker.Split(topic.Body); + var chunks = new List(chunkTexts.Count); + for (var i = 0; i < chunkTexts.Count; i++) + { + var embedding = await _embeddingProvider.EmbedAsync(chunkTexts[i], ct); + chunks.Add(new HelpTopicChunk(Guid.CreateVersion7(), topic.Id, i, chunkTexts[i], embedding, _embeddingProvider.ModelId, topic.ContentHash)); + } + + topic.ReplaceChunks(chunks); + await _repository.SaveTopicAsync(topic, ct); + } + + var chunkCount = topics.Sum(t => t.Chunks.Count); + await ctx.Session.WriteLineAsync($"Rebuilt the help index: {topics.Count} topic(s), {chunkCount} chunk(s).", ct); + } +} diff --git a/src/SharpMud.Engine/Commands/Builtin/Admin/HelpTopicEditCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Admin/HelpTopicEditCommand.cs new file mode 100644 index 0000000..6d7166b --- /dev/null +++ b/src/SharpMud.Engine/Commands/Builtin/Admin/HelpTopicEditCommand.cs @@ -0,0 +1,52 @@ +using SharpMud.Engine.Help; + +namespace SharpMud.Engine.Commands.Builtin.Admin; + +/// +/// The helptopic <key> <body> command (, ADR-0010) - creates a new +/// or overwrites an existing one's , keyed by exact, case-insensitive match. This is the only authoring path for help +/// content - no file-based alternative (see ADR-0010's Decision Outcome). +/// Aliases/keywords aren't settable via this command yet - deliberately out +/// of v1 scope, see PLAN-0010's Open questions. +/// +public sealed class HelpTopicEditCommand : ICommand +{ + private readonly IHelpRepository _repository; + + /// Creates the command, saving through . + public HelpTopicEditCommand(IHelpRepository repository) + { + _repository = repository; + } + + /// + public string Verb => "helptopic"; + + /// + public IReadOnlyList Aliases { get; } = []; + + /// + public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + if (ctx.Args.Count < 2) + { + await ctx.Session.WriteLineAsync("Usage: helptopic ", ct); + return; + } + + var key = ctx.Args[0]; + var body = string.Join(' ', ctx.Args.Skip(1)); + + var topic = await _repository.FindByNameOrAliasAsync(key, ct) ?? new HelpTopic { Id = HelpTopicId.New(), Key = key }; + topic.Key = key; + topic.Body = body; + topic.Touch(); + + await _repository.SaveTopicAsync(topic, ct); + + await ctx.Session.WriteLineAsync($"Help topic '{key}' saved.", ct); + } +} diff --git a/src/SharpMud.Engine/Commands/Builtin/BuiltinCommands.cs b/src/SharpMud.Engine/Commands/Builtin/BuiltinCommands.cs index bf909cd..5bfdf76 100644 --- a/src/SharpMud.Engine/Commands/Builtin/BuiltinCommands.cs +++ b/src/SharpMud.Engine/Commands/Builtin/BuiltinCommands.cs @@ -1,4 +1,5 @@ using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Help; namespace SharpMud.Engine.Commands.Builtin; @@ -13,7 +14,7 @@ namespace SharpMud.Engine.Commands.Builtin; // themselves" visibly separate. public static class BuiltinCommands { - public static void RegisterAll(ICommandRegistry registry) + public static void RegisterAll(ICommandRegistry registry, IHelpRepository helpRepository, IHelpSearchIndex helpSearchIndex) { registry.RegisterOpen(new MoveCommand(Direction.North, "north", ["n"])); registry.RegisterOpen(new MoveCommand(Direction.South, "south", ["s"])); @@ -40,6 +41,6 @@ public static void RegisterAll(ICommandRegistry registry) registry.RegisterOpen(new RemoveCommand()); registry.RegisterOpen(new InventoryCommand()); registry.RegisterOpen(new GiveCommand()); - registry.RegisterOpen(new HelpCommand(registry)); + registry.RegisterOpen(new HelpCommand(registry, helpRepository, helpSearchIndex)); } } diff --git a/src/SharpMud.Engine/Commands/Builtin/HelpCommand.cs b/src/SharpMud.Engine/Commands/Builtin/HelpCommand.cs index a2a0785..28f5660 100644 --- a/src/SharpMud.Engine/Commands/Builtin/HelpCommand.cs +++ b/src/SharpMud.Engine/Commands/Builtin/HelpCommand.cs @@ -1,18 +1,82 @@ using SharpMud.Engine.Behaviors; +using SharpMud.Engine.Help; namespace SharpMud.Engine.Commands.Builtin; -public sealed class HelpCommand(ICommandRegistry registry) : ICommand +/// +/// The help command. With no arguments, lists every command the +/// actor can see (unchanged behavior). With an argument, resolves a +/// via ADR-0010's three-tier pipeline: exact +/// / match, then +/// match, then +/// semantic search - falling back to "no help topic found" only once all +/// three miss, never a weak guess. +/// +public sealed class HelpCommand : ICommand { + private readonly ICommandRegistry _registry; + private readonly IHelpRepository _helpRepository; + private readonly IHelpSearchIndex _helpSearchIndex; + + /// Creates a help command listing commands from and resolving topics via /. + public HelpCommand(ICommandRegistry registry, IHelpRepository helpRepository, IHelpSearchIndex helpSearchIndex) + { + _registry = registry; + _helpRepository = helpRepository; + _helpSearchIndex = helpSearchIndex; + } + + /// public string Verb => "help"; + + /// public IReadOnlyList Aliases { get; } = []; + /// public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) + { + if (ctx.Args.Count > 0) + { + await ExecuteTopicLookupAsync(ctx, ct); + return; + } + + await ExecuteCommandListingAsync(ctx, ct); + } + + private async Task ExecuteTopicLookupAsync(CommandContext ctx, CancellationToken ct) + { + var query = string.Join(' ', ctx.Args); + + var topic = await _helpRepository.FindByNameOrAliasAsync(query, ct); + + if (topic is null) + { + var keywordMatches = await _helpRepository.FindByKeywordAsync(query, ct); + topic = keywordMatches.FirstOrDefault(); + } + + if (topic is null) + { + var hits = await _helpSearchIndex.SearchAsync(query, ct); + topic = hits.FirstOrDefault()?.Topic; + } + + if (topic is null) + { + await ctx.Session.WriteLineAsync($"No help topic found for '{query}'.", ct); + return; + } + + await ctx.Session.WriteLineAsync(topic.Body, ct); + } + + private async Task ExecuteCommandListingAsync(CommandContext ctx, CancellationToken ct) { await ctx.Session.WriteLineAsync("Available commands:", ct); var actorRoles = ctx.Actor.FindBehavior()?.Roles ?? SecurityRole.None; - foreach (var command in registry.Commands.OrderBy(c => c.Verb, StringComparer.Ordinal)) + foreach (var command in _registry.Commands.OrderBy(c => c.Verb, StringComparer.Ordinal)) { // RoleGuardedCommand passes Verb/Aliases straight through from // the command it wraps, so without this check every admin diff --git a/src/SharpMud.Engine/Help/CosineHelpSearchIndex.cs b/src/SharpMud.Engine/Help/CosineHelpSearchIndex.cs new file mode 100644 index 0000000..43b4ab8 --- /dev/null +++ b/src/SharpMud.Engine/Help/CosineHelpSearchIndex.cs @@ -0,0 +1,65 @@ +namespace SharpMud.Engine.Help; + +/// +/// Brute-force cosine-similarity (ADR-0010) - +/// loads every / through +/// and compares in app code; no vector-search +/// dependency. Fine at help-topic corpus scale (dozens-hundreds of topics); +/// revisit (e.g. an ANN-backed index) if that scale ever changes - see +/// ADR-0010's Negative Consequences. +/// +public sealed class CosineHelpSearchIndex : IHelpSearchIndex +{ + private readonly IHelpRepository _repository; + private readonly IEmbeddingProvider _embeddingProvider; + + /// Creates a search index reading topics through and embedding queries via . + public CosineHelpSearchIndex(IHelpRepository repository, IEmbeddingProvider embeddingProvider) + { + _repository = repository; + _embeddingProvider = embeddingProvider; + } + + /// + public async Task> SearchAsync(string query, CancellationToken ct) + { + var queryVector = await _embeddingProvider.EmbedAsync(query, ct); + var topics = await _repository.GetAllTopicsAsync(ct); + var relevanceThreshold = _embeddingProvider.RelevanceThreshold; + + var bestByTopic = new Dictionary(); + foreach (var topic in topics) + { + foreach (var chunk in topic.Chunks) + { + var score = CosineSimilarity(queryVector, chunk.Embedding); + if (score < relevanceThreshold) + continue; + + if (!bestByTopic.TryGetValue(topic.Id, out var existing) || score > existing.Score) + bestByTopic[topic.Id] = new HelpSearchHit(topic, score); + } + } + + return bestByTopic.Values.OrderByDescending(hit => hit.Score).ToList(); + } + + private static double CosineSimilarity(float[] a, float[] b) + { + if (a.Length != b.Length) + return 0; + + double dot = 0, magnitudeA = 0, magnitudeB = 0; + for (var i = 0; i < a.Length; i++) + { + dot += a[i] * b[i]; + magnitudeA += a[i] * a[i]; + magnitudeB += b[i] * b[i]; + } + + if (magnitudeA == 0 || magnitudeB == 0) + return 0; + + return dot / (Math.Sqrt(magnitudeA) * Math.Sqrt(magnitudeB)); + } +} diff --git a/src/SharpMud.Engine/Help/HelpContentHashing.cs b/src/SharpMud.Engine/Help/HelpContentHashing.cs new file mode 100644 index 0000000..e90e28c --- /dev/null +++ b/src/SharpMud.Engine/Help/HelpContentHashing.cs @@ -0,0 +1,13 @@ +using System.Security.Cryptography; +using System.Text; + +namespace SharpMud.Engine.Help; + +// Shared by HelpTopic.ContentHash and anything comparing it against a +// HelpTopicChunk.SourceContentHash - kept as one static function rather than +// duplicated so the exact hash inputs/encoding can't drift between the two. +internal static class HelpContentHashing +{ + public static string Compute(string text) => + Convert.ToHexStringLower(SHA256.HashData(Encoding.UTF8.GetBytes(text))); +} diff --git a/src/SharpMud.Engine/Help/HelpSearchHit.cs b/src/SharpMud.Engine/Help/HelpSearchHit.cs new file mode 100644 index 0000000..1c11d8c --- /dev/null +++ b/src/SharpMud.Engine/Help/HelpSearchHit.cs @@ -0,0 +1,6 @@ +namespace SharpMud.Engine.Help; + +/// One result - a matched topic and its relevance score. +/// The matched topic. +/// Cosine similarity in [-1, 1] (in practice [0, 1] for non-negative embeddings) between the query and this topic's best-matching chunk. +public sealed record HelpSearchHit(HelpTopic Topic, double Score); diff --git a/src/SharpMud.Engine/Help/HelpTopic.cs b/src/SharpMud.Engine/Help/HelpTopic.cs new file mode 100644 index 0000000..69f8c18 --- /dev/null +++ b/src/SharpMud.Engine/Help/HelpTopic.cs @@ -0,0 +1,69 @@ +namespace SharpMud.Engine.Help; + +/// +/// A help topic - the aggregate root of sharp-mud's help system (ADR-0010). +/// Independently addressable, own repository (), +/// same "aggregate root gets exactly one repository" shape as Thing/ +/// IThingRepository - not a Thing/Behavior itself. +/// Authored and edited only via an in-game admin command; there is no +/// file-based content path (see ADR-0010's Decision Outcome for why, and +/// docs/research/wheelmud-findings.md §12 for the WheelMUD precedent +/// this deliberately deviates from). +/// +public sealed class HelpTopic +{ + private readonly List _aliases = []; + private readonly List _keywords = []; + private readonly List _chunks = []; + + /// Stable identity, independent of (which can be renamed via edit). + public required HelpTopicId Id { get; init; } + + /// The canonical, primary name this topic is looked up by - e.g. help wizard. Editable directly, like Thing.Name/Thing.Description (both plain settable properties, the established shape for admin-command-edited content in this codebase). + public required string Key { get; set; } + + /// Free-text grouping, e.g. "classes". Empty string means uncategorized. + public string Category { get; set; } = ""; + + /// The authored help text shown to a player on lookup - the canonical source of truth (ADR-0010); s are a rebuildable derivative of this, never the reverse. + public string Body { get; set; } = ""; + + /// When ///aliases/keywords last changed. Only mutated via . + public DateTimeOffset UpdatedAtUtc { get; private set; } + + /// Additional names this topic is looked up by, besides . Mutate via . + public IReadOnlyList Aliases => _aliases; + + /// Free-text keywords this topic matches on the keyword-lookup tier (ADR-0010's pipeline, tier two). Mutate via . + public IReadOnlyList Keywords => _keywords; + + /// This topic's current embedding chunks - a rebuildable index over , not authoritative content. Replaced wholesale via , never edited individually. + public IReadOnlyList Chunks => _chunks; + + /// SHA-256 hex digest of - compared against each to detect a stale embedding index. Derived, not persisted directly. + public string ContentHash => HelpContentHashing.Compute(Body); + + /// Replaces this topic's alias set wholesale. + public void SetAliases(IEnumerable aliases) + { + _aliases.Clear(); + _aliases.AddRange(aliases); + } + + /// Replaces this topic's keyword set wholesale. + public void SetKeywords(IEnumerable keywords) + { + _keywords.Clear(); + _keywords.AddRange(keywords); + } + + /// Records that this topic's content changed just now - called by the admin edit command after making its changes. + public void Touch() => UpdatedAtUtc = DateTimeOffset.UtcNow; + + /// Replaces this topic's wholesale - called by helpindex rebuild. Chunks are always regenerated in full from the current , never patched individually. + public void ReplaceChunks(IEnumerable chunks) + { + _chunks.Clear(); + _chunks.AddRange(chunks); + } +} diff --git a/src/SharpMud.Engine/Help/HelpTopicChunk.cs b/src/SharpMud.Engine/Help/HelpTopicChunk.cs new file mode 100644 index 0000000..1e8c864 --- /dev/null +++ b/src/SharpMud.Engine/Help/HelpTopicChunk.cs @@ -0,0 +1,23 @@ +namespace SharpMud.Engine.Help; + +/// +/// One embedded slice of a 's +/// (ADR-0010) - a value object, always regenerated wholesale by helpindex +/// rebuild (see ), never edited in +/// place. +/// +/// Row identity - needed only because it's its own persisted table row, not a domain concept callers reason about. +/// The owning . +/// Position within the topic's chunk sequence. +/// The chunk's source text, as embedded. +/// The embedding vector produced by for . +/// Which produced - lets a future rebuild detect a model change, not just a content change. +/// The owning topic's at embed time - compared against the topic's current hash to detect a stale index. +public sealed record HelpTopicChunk( + Guid Id, + HelpTopicId HelpTopicId, + int ChunkIndex, + string Text, + float[] Embedding, + string EmbeddingModelId, + string SourceContentHash); diff --git a/src/SharpMud.Engine/Help/HelpTopicChunker.cs b/src/SharpMud.Engine/Help/HelpTopicChunker.cs new file mode 100644 index 0000000..56fa5e9 --- /dev/null +++ b/src/SharpMud.Engine/Help/HelpTopicChunker.cs @@ -0,0 +1,22 @@ +namespace SharpMud.Engine.Help; + +/// +/// Splits a into per-paragraph chunks (blank +/// lines as boundaries) - used by helpindex rebuild to decide what +/// gets embedded. A static function, not a service, since it's pure text +/// splitting with no dependency. +/// +public static class HelpTopicChunker +{ + /// Splits on blank lines into non-empty, trimmed paragraphs. Empty/whitespace-only input yields no chunks. + public static IReadOnlyList Split(string body) + { + if (string.IsNullOrWhiteSpace(body)) + return []; + + return body + .Split("\n\n", StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries) + .Where(p => p.Length > 0) + .ToList(); + } +} diff --git a/src/SharpMud.Engine/Help/HelpTopicId.cs b/src/SharpMud.Engine/Help/HelpTopicId.cs new file mode 100644 index 0000000..1d3094d --- /dev/null +++ b/src/SharpMud.Engine/Help/HelpTopicId.cs @@ -0,0 +1,11 @@ +namespace SharpMud.Engine.Help; + +/// Stable identity for a - same wrapper-struct shape as ThingId. +public readonly record struct HelpTopicId(Guid Value) +{ + /// Generates a new, time-ordered id. + public static HelpTopicId New() => new(Guid.CreateVersion7()); + + /// + public override string ToString() => Value.ToString(); +} diff --git a/src/SharpMud.Engine/Help/IEmbeddingProvider.cs b/src/SharpMud.Engine/Help/IEmbeddingProvider.cs new file mode 100644 index 0000000..9ad47ec --- /dev/null +++ b/src/SharpMud.Engine/Help/IEmbeddingProvider.cs @@ -0,0 +1,32 @@ +namespace SharpMud.Engine.Help; + +/// +/// Turns text into an embedding vector - the abstraction ADR-0010 puts +/// between the help-search pipeline and whatever produces embeddings, so a +/// real model can be swapped in later behind this interface without +/// touching or any command. The default +/// implementation () is a deterministic +/// placeholder, not a production-quality semantic model. +/// +public interface IEmbeddingProvider +{ + /// Identifies which model/version produced an embedding - stored on so a future model change can be detected, not just a content change. + string ModelId { get; } + + /// + /// Below this cosine score, treats a + /// match as too weak to show - "no help topic found" beats an + /// unrelated guess (ADR-0010). Owned by the provider, not the search + /// index, because different embedding models produce cosine scores on + /// very different scales - e.g. a sparse hashed-bag-of-words vector + /// scores near 0 for unrelated text, while a dense sentence-embedding + /// model can score 0.3+ for completely unrelated text and 0.6+ for a + /// real match (measured empirically for + /// SmartComponents.LocalEmbeddings' default model - see + /// ADR-0011); one hardcoded threshold can't be right for both. + /// + double RelevanceThreshold { get; } + + /// Embeds into a fixed-dimension vector. + Task EmbedAsync(string text, CancellationToken ct); +} diff --git a/src/SharpMud.Engine/Help/IHelpRepository.cs b/src/SharpMud.Engine/Help/IHelpRepository.cs new file mode 100644 index 0000000..af14654 --- /dev/null +++ b/src/SharpMud.Engine/Help/IHelpRepository.cs @@ -0,0 +1,25 @@ +namespace SharpMud.Engine.Help; + +/// +/// The aggregate-root repository for (ADR-0010) - +/// one repository per independently-addressable aggregate root, matching +/// IThingRepository's shape (see docs/persistence.md: no +/// generic/per-concept repositories). +/// +public interface IHelpRepository +{ + /// Finds a topic by exact, case-insensitive or match - tier one of ADR-0010's lookup pipeline. + Task FindByNameOrAliasAsync(string name, CancellationToken ct); + + /// Finds every topic whose contains an exact, case-insensitive match - tier two of ADR-0010's lookup pipeline. + Task> FindByKeywordAsync(string keyword, CancellationToken ct); + + /// Every topic, each with its current loaded - used by (tier three) and by helpindex rebuild. + Task> GetAllTopicsAsync(CancellationToken ct); + + /// Creates or overwrites , including its current , as one unit. + Task SaveTopicAsync(HelpTopic topic, CancellationToken ct); + + /// Deletes a topic and its chunks. No-op if doesn't exist. + Task DeleteTopicAsync(HelpTopicId id, CancellationToken ct); +} diff --git a/src/SharpMud.Engine/Help/IHelpSearchIndex.cs b/src/SharpMud.Engine/Help/IHelpSearchIndex.cs new file mode 100644 index 0000000..ef3f383 --- /dev/null +++ b/src/SharpMud.Engine/Help/IHelpSearchIndex.cs @@ -0,0 +1,18 @@ +namespace SharpMud.Engine.Help; + +/// +/// Semantic search over content (ADR-0010) - tier +/// three of the help lookup pipeline, only ever consulted after exact-name/ +/// alias and keyword lookup both miss. Implementations apply their own +/// relevance threshold and must return an empty list rather than a weak +/// guess when nothing clears it - "no help topic found" beats an unrelated +/// answer. The default implementation () +/// is storage-agnostic (reads through only), +/// so a future vector-storage swap (e.g. an ANN-backed index) replaces this +/// interface's implementation without touching any command. +/// +public interface IHelpSearchIndex +{ + /// Returns the best-matching topics for , ordered by descending relevance, above whatever threshold this implementation enforces. Empty, not an exception or a weak guess, when nothing qualifies. + Task> SearchAsync(string query, CancellationToken ct); +} diff --git a/src/SharpMud.Engine/Help/StubEmbeddingProvider.cs b/src/SharpMud.Engine/Help/StubEmbeddingProvider.cs new file mode 100644 index 0000000..ec7d30a --- /dev/null +++ b/src/SharpMud.Engine/Help/StubEmbeddingProvider.cs @@ -0,0 +1,63 @@ +namespace SharpMud.Engine.Help; + +/// +/// Deterministic placeholder (ADR-0010) - +/// feature-hashes lowercase word tokens into a fixed-size vector (the +/// "hashing trick"), with no external dependency or model asset. Retrieval +/// quality reflects literal word overlap only (no synonym/semantic +/// understanding - "wizard" and "mage" share no signal); this exists to +/// validate the pipeline/abstraction end-to-end with fully deterministic, +/// testable output, not to be the long-term provider. A real model (local +/// or API-based) swaps in behind later - +/// see ADR-0010's Negative Consequences. +/// +public sealed class StubEmbeddingProvider : IEmbeddingProvider +{ + private const int Dimensions = 128; + + private static readonly char[] TokenSeparators = [' ', '\t', '\n', '\r', '.', ',', '!', '?', ';', ':', '"', '\'']; + + /// + public string ModelId => "stub-hashed-bow-v1"; + + /// Tuned empirically for this provider's sparse hashed-bag-of-words vectors, which score near 0 for genuinely unrelated text. + public double RelevanceThreshold => 0.15; + + /// + public Task EmbedAsync(string text, CancellationToken ct) + { + var vector = new float[Dimensions]; + foreach (var word in text.ToLowerInvariant().Split(TokenSeparators, StringSplitOptions.RemoveEmptyEntries)) + vector[(int)(Fnv1aHash(word) % Dimensions)] += 1f; + + Normalize(vector); + return Task.FromResult(vector); + } + + // FNV-1a - deterministic across processes/runs, unlike string.GetHashCode + // (randomized per-process by default in .NET), which this needs to be + // reproducible: the same word must always hash to the same bucket, both + // within one run (query vs. chunk) and across separate rebuild/search + // calls. + private static uint Fnv1aHash(string text) + { + var hash = 2166136261u; + foreach (var ch in text) + { + hash ^= ch; + hash *= 16777619u; + } + + return hash; + } + + private static void Normalize(float[] vector) + { + var magnitude = MathF.Sqrt(vector.Sum(v => v * v)); + if (magnitude == 0f) + return; + + for (var i = 0; i < vector.Length; i++) + vector[i] /= magnitude; + } +} diff --git a/src/SharpMud.Hosting/ServiceCollectionExtensions.cs b/src/SharpMud.Hosting/ServiceCollectionExtensions.cs index f6ceee9..26a2aff 100644 --- a/src/SharpMud.Hosting/ServiceCollectionExtensions.cs +++ b/src/SharpMud.Hosting/ServiceCollectionExtensions.cs @@ -3,6 +3,7 @@ using SharpMud.Engine.Commands; using SharpMud.Engine.Commands.Builtin; using SharpMud.Engine.Core; +using SharpMud.Engine.Help; using SharpMud.Engine.Sessions; using SharpMud.Engine.Ticking; @@ -27,7 +28,7 @@ public static IServiceCollection AddSharpMudRuleset(this IServiceCollection serv services.AddSingleton(sp => { var registry = new CommandRegistry(); - BuiltinCommands.RegisterAll(registry); + BuiltinCommands.RegisterAll(registry, sp.GetRequiredService(), sp.GetRequiredService()); registerRuleset(sp, registry); return registry; }); diff --git a/src/SharpMud.Persistence.Sqlite/ServiceCollectionExtensions.cs b/src/SharpMud.Persistence.Sqlite/ServiceCollectionExtensions.cs index 75809c9..f8a0668 100644 --- a/src/SharpMud.Persistence.Sqlite/ServiceCollectionExtensions.cs +++ b/src/SharpMud.Persistence.Sqlite/ServiceCollectionExtensions.cs @@ -1,19 +1,32 @@ using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; using SharpMud.Engine.Core; +using SharpMud.Engine.Help; using SharpMud.Persistence; namespace SharpMud.Persistence.Sqlite; public static class ServiceCollectionExtensions { - /// Registers backed by SQLite at . + /// + /// Registers and + /// (ADR-0010), both backed by SQLite at , plus + /// the default help-search stack (/ + /// ) - registered here, not left + /// opt-in, since help (unlike helptopic/helpindex) + /// is always part of , + /// the same "always-available infrastructure" shape already has here. + /// public static IServiceCollection AddSharpMudSqlitePersistence(this IServiceCollection services, string dbPath) { ArgumentNullException.ThrowIfNull(dbPath); services.AddDbContextFactory(options => options.UseSqlite($"Data Source={dbPath}")); services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); return services; diff --git a/src/SharpMud.Persistence/Configurations/HelpTopicChunkConfiguration.cs b/src/SharpMud.Persistence/Configurations/HelpTopicChunkConfiguration.cs new file mode 100644 index 0000000..0ace950 --- /dev/null +++ b/src/SharpMud.Persistence/Configurations/HelpTopicChunkConfiguration.cs @@ -0,0 +1,38 @@ +using System.Runtime.InteropServices; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Metadata.Builders; +using SharpMud.Engine.Help; + +namespace SharpMud.Persistence.Configurations; + +// HelpTopicChunk is a record (constructor-bound materialization, same +// mechanism ThingId already relies on for Thing.Id) - its own table with an +// explicit HelpTopicId FK, not an EF navigation collection off HelpTopic. +// HelpRepository loads/saves it manually, mirroring how ThingRepository +// handles the Behaviors table. +public sealed class HelpTopicChunkConfiguration : IEntityTypeConfiguration +{ + public void Configure(EntityTypeBuilder builder) + { + builder.ToTable("HelpTopicChunks"); + builder.HasKey(x => x.Id); + builder.Property(x => x.HelpTopicId).HasConversion(id => id.Value, value => new HelpTopicId(value)); + builder.HasIndex(x => x.HelpTopicId); + builder.Property(x => x.Text).IsRequired(); + builder.Property(x => x.EmbeddingModelId).IsRequired(); + builder.Property(x => x.SourceContentHash).IsRequired(); + + // Stored as a BLOB (ADR-0010) rather than a delimited string - a + // straight little-endian byte reinterpretation of the float[], not + // meant to be portable across machine architectures, only read back + // by this same process/DB. Factored into static methods (not inline + // Span usage) because a ref struct value can't appear inside the + // expression tree HasConversion compiles its lambdas into (CS8640). + builder.Property(x => x.Embedding) + .HasConversion(v => Serialize(v), v => Deserialize(v)); + } + + private static byte[] Serialize(float[] vector) => MemoryMarshal.AsBytes(vector.AsSpan()).ToArray(); + + private static float[] Deserialize(byte[] bytes) => MemoryMarshal.Cast(bytes).ToArray(); +} diff --git a/src/SharpMud.Persistence/Configurations/HelpTopicConfiguration.cs b/src/SharpMud.Persistence/Configurations/HelpTopicConfiguration.cs new file mode 100644 index 0000000..29c6a87 --- /dev/null +++ b/src/SharpMud.Persistence/Configurations/HelpTopicConfiguration.cs @@ -0,0 +1,49 @@ +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Metadata.Builders; +using SharpMud.Engine.Help; + +namespace SharpMud.Persistence.Configurations; + +// Aliases/Keywords are List-backed IReadOnlyList properties +// with no public setter (see HelpTopic) - EF binds directly to the backing +// field (PropertyAccessMode.Field) instead of requiring a settable property, +// same "expose IReadOnlyList, mutate via a named method" shape +// coding-standards.md requires for collections on domain entities. +public sealed class HelpTopicConfiguration : IEntityTypeConfiguration +{ + // ASCII "unit separator" (0x1F) - won't collide with real alias/keyword text. + private const char Separator = '\u001F'; + + public void Configure(EntityTypeBuilder builder) + { + builder.ToTable("HelpTopics"); + builder.HasKey(x => x.Id); + builder.Property(x => x.Id).HasConversion(id => id.Value, value => new HelpTopicId(value)); + builder.Property(x => x.Key).IsRequired(); + builder.Property(x => x.Category).IsRequired(); + builder.Property(x => x.Body).IsRequired(); + builder.Property(x => x.UpdatedAtUtc).IsRequired(); + + builder.Property(x => x.Aliases) + .HasConversion(v => Join(v), v => Split(v)) + .Metadata.SetPropertyAccessMode(PropertyAccessMode.Field); + + builder.Property(x => x.Keywords) + .HasConversion(v => Join(v), v => Split(v)) + .Metadata.SetPropertyAccessMode(PropertyAccessMode.Field); + + // HelpTopicChunk is its own table with an explicit HelpTopicId FK, + // loaded/saved manually by HelpRepository (mirrors ThingRepository's + // Behaviors handling) rather than an EF navigation collection. + builder.Ignore(x => x.Chunks); + builder.Ignore(x => x.ContentHash); // derived from Body, not stored + } + + // Factored out of the HasConversion lambdas above - a collection + // expression ([]) can't appear inside an expression tree (CS9175), + // which is what EF compiles a HasConversion lambda into. + private static string Join(IReadOnlyList values) => string.Join(Separator, values); + + private static List Split(string value) => + value.Length == 0 ? new List() : value.Split(Separator, StringSplitOptions.RemoveEmptyEntries).ToList(); +} diff --git a/src/SharpMud.Persistence/GameDbContext.cs b/src/SharpMud.Persistence/GameDbContext.cs index 923858a..37c197d 100644 --- a/src/SharpMud.Persistence/GameDbContext.cs +++ b/src/SharpMud.Persistence/GameDbContext.cs @@ -1,5 +1,6 @@ using Microsoft.EntityFrameworkCore; using SharpMud.Engine.Core; +using SharpMud.Engine.Help; namespace SharpMud.Persistence; @@ -16,6 +17,13 @@ public sealed class GameDbContext( public DbSet Things => Set(); public DbSet Behaviors => Set(); + // HelpTopic/HelpTopicChunk (ADR-0010) - unlike Thing/Behavior, these have + // no Rehydration/event-firing concerns, so they use EF Core normally + // (real conversions, no shadow FKs, no manual reconstruction) rather than + // ThingRepository's hand-rolled pattern. See HelpRepository. + public DbSet HelpTopics => Set(); + public DbSet HelpTopicChunks => Set(); + protected override void OnModelCreating(ModelBuilder modelBuilder) { // Picks up every IEntityTypeConfiguration in this assembly - diff --git a/src/SharpMud.Persistence/HelpRepository.cs b/src/SharpMud.Persistence/HelpRepository.cs new file mode 100644 index 0000000..dc9fe37 --- /dev/null +++ b/src/SharpMud.Persistence/HelpRepository.cs @@ -0,0 +1,77 @@ +using Microsoft.EntityFrameworkCore; +using SharpMud.Engine.Help; + +namespace SharpMud.Persistence; + +// Loads the full HelpTopic/HelpTopicChunk corpus per call, same "load +// everything" shape ThingRepository already uses - fine at help-topic scale +// (dozens-hundreds of topics), see ADR-0010's Negative Consequences. Fresh +// GameDbContext per call (IDbContextFactory), same reason as +// ThingRepository: DbContext isn't thread-safe, and re-adding the same +// tracked instances across repeated saves would throw. +public sealed class HelpRepository(IDbContextFactory dbContextFactory) : IHelpRepository +{ + public async Task FindByNameOrAliasAsync(string name, CancellationToken ct) + { + var topics = await GetAllTopicsAsync(ct); + return topics.FirstOrDefault(t => + string.Equals(t.Key, name, StringComparison.OrdinalIgnoreCase) || + t.Aliases.Any(a => string.Equals(a, name, StringComparison.OrdinalIgnoreCase))); + } + + public async Task> FindByKeywordAsync(string keyword, CancellationToken ct) + { + var topics = await GetAllTopicsAsync(ct); + return topics + .Where(t => t.Keywords.Any(k => string.Equals(k, keyword, StringComparison.OrdinalIgnoreCase))) + .ToList(); + } + + public async Task> GetAllTopicsAsync(CancellationToken ct) + { + await using var context = await dbContextFactory.CreateDbContextAsync(ct); + + var topics = await context.HelpTopics.AsNoTracking().ToListAsync(ct); + var chunksByTopic = (await context.HelpTopicChunks.AsNoTracking().ToListAsync(ct)) + .ToLookup(c => c.HelpTopicId); + + foreach (var topic in topics) + topic.ReplaceChunks(chunksByTopic[topic.Id].OrderBy(c => c.ChunkIndex)); + + return topics; + } + + // Delete-then-insert, two SaveChangesAsync calls - same PK-conflict + // avoidance as ThingRepository.SaveTreeAsync (re-adding a still-tracked + // instance in one batch throws). + public async Task SaveTopicAsync(HelpTopic topic, CancellationToken ct) + { + await using var context = await dbContextFactory.CreateDbContextAsync(ct); + + var existingTopic = await context.HelpTopics.Where(t => t.Id == topic.Id).ToListAsync(ct); + context.HelpTopics.RemoveRange(existingTopic); + + var existingChunks = await context.HelpTopicChunks.Where(c => c.HelpTopicId == topic.Id).ToListAsync(ct); + context.HelpTopicChunks.RemoveRange(existingChunks); + + await context.SaveChangesAsync(ct); + + context.HelpTopics.Add(topic); + context.HelpTopicChunks.AddRange(topic.Chunks); + + await context.SaveChangesAsync(ct); + } + + public async Task DeleteTopicAsync(HelpTopicId id, CancellationToken ct) + { + await using var context = await dbContextFactory.CreateDbContextAsync(ct); + + var chunks = await context.HelpTopicChunks.Where(c => c.HelpTopicId == id).ToListAsync(ct); + context.HelpTopicChunks.RemoveRange(chunks); + + var topics = await context.HelpTopics.Where(t => t.Id == id).ToListAsync(ct); + context.HelpTopics.RemoveRange(topics); + + await context.SaveChangesAsync(ct); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/HelpIndexRebuildCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/HelpIndexRebuildCommandTests.cs new file mode 100644 index 0000000..0eceb8f --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/HelpIndexRebuildCommandTests.cs @@ -0,0 +1,57 @@ +using SharpMud.Engine.Commands; +using SharpMud.Engine.Commands.Builtin.Admin; +using SharpMud.Engine.Core; +using SharpMud.Engine.Help; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Engine.Tests.Commands.Builtin.Admin; + +public sealed class HelpIndexRebuildCommandTests +{ + private static (Thing Actor, World World, ISession Session) MakeActor() + { + var world = new World(); + var actor = new Thing { Id = ThingId.New(), Name = "Builder" }; + world.Register(actor); + return (actor, world, Substitute.For()); + } + + [Fact] + public async Task ExecuteAsync_ReplacesChunks_ForEveryTopic() + { + var (actor, world, session) = MakeActor(); + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "How to become a wizard." }; + var repository = Substitute.For(); + repository.GetAllTopicsAsync(Arg.Any()).Returns([topic]); + var embeddingProvider = Substitute.For(); + embeddingProvider.ModelId.Returns("stub-hashed-bow-v1"); + embeddingProvider.EmbedAsync(Arg.Any(), Arg.Any()).Returns([1f, 0f]); + + var sut = new HelpIndexRebuildCommand(repository, embeddingProvider); + var ctx = new CommandContext(actor, actor, ["rebuild"], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + topic.Chunks.Should().ContainSingle(); + topic.Chunks[0].SourceContentHash.Should().Be(topic.ContentHash); + topic.Chunks[0].EmbeddingModelId.Should().Be("stub-hashed-bow-v1"); + await repository.Received(1).SaveTopicAsync(topic, Arg.Any()); + await session.Received(1).WriteLineAsync("Rebuilt the help index: 1 topic(s), 1 chunk(s).", Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_SendsUsageMessage_WhenArgumentIsNotRebuild() + { + var (actor, world, session) = MakeActor(); + var repository = Substitute.For(); + var embeddingProvider = Substitute.For(); + + var sut = new HelpIndexRebuildCommand(repository, embeddingProvider); + var ctx = new CommandContext(actor, actor, ["nonsense"], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await session.Received(1).WriteLineAsync("Usage: helpindex rebuild", Arg.Any()); + await repository.DidNotReceiveWithAnyArgs().GetAllTopicsAsync(Arg.Any()); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/HelpTopicEditCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/HelpTopicEditCommandTests.cs new file mode 100644 index 0000000..0334000 --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/HelpTopicEditCommandTests.cs @@ -0,0 +1,69 @@ +using SharpMud.Engine.Commands; +using SharpMud.Engine.Commands.Builtin.Admin; +using SharpMud.Engine.Core; +using SharpMud.Engine.Help; +using SharpMud.Engine.Sessions; + +namespace SharpMud.Engine.Tests.Commands.Builtin.Admin; + +public sealed class HelpTopicEditCommandTests +{ + private static (Thing Actor, World World, ISession Session) MakeActor() + { + var world = new World(); + var actor = new Thing { Id = ThingId.New(), Name = "Builder" }; + world.Register(actor); + return (actor, world, Substitute.For()); + } + + [Fact] + public async Task ExecuteAsync_CreatesNewTopic_WhenNoneExists() + { + var (actor, world, session) = MakeActor(); + var repository = Substitute.For(); + repository.FindByNameOrAliasAsync("wizard", Arg.Any()).Returns((HelpTopic?)null); + + var sut = new HelpTopicEditCommand(repository); + var ctx = new CommandContext(actor, actor, ["wizard", "How", "to", "become", "a", "wizard."], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await repository.Received(1).SaveTopicAsync( + Arg.Is(t => t.Key == "wizard" && t.Body == "How to become a wizard."), + Arg.Any()); + await session.Received(1).WriteLineAsync("Help topic 'wizard' saved.", Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_OverwritesBody_WhenTopicAlreadyExists() + { + var (actor, world, session) = MakeActor(); + var existing = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "Old text." }; + var repository = Substitute.For(); + repository.FindByNameOrAliasAsync("wizard", Arg.Any()).Returns(existing); + + var sut = new HelpTopicEditCommand(repository); + var ctx = new CommandContext(actor, actor, ["wizard", "New", "text."], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await repository.Received(1).SaveTopicAsync( + Arg.Is(t => t.Id == existing.Id && t.Body == "New text."), + Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_SendsUsageMessage_WhenMissingBody() + { + var (actor, world, session) = MakeActor(); + var repository = Substitute.For(); + + var sut = new HelpTopicEditCommand(repository); + var ctx = new CommandContext(actor, actor, ["wizard"], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await session.Received(1).WriteLineAsync("Usage: helptopic ", Arg.Any()); + await repository.DidNotReceiveWithAnyArgs().SaveTopicAsync(default!, Arg.Any()); + } +} diff --git a/tests/SharpMud.Engine.Tests/Commands/HelpCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/HelpCommandTests.cs index 12d3a4c..4e3067a 100644 --- a/tests/SharpMud.Engine.Tests/Commands/HelpCommandTests.cs +++ b/tests/SharpMud.Engine.Tests/Commands/HelpCommandTests.cs @@ -2,6 +2,7 @@ using SharpMud.Engine.Commands; using SharpMud.Engine.Commands.Builtin; using SharpMud.Engine.Core; +using SharpMud.Engine.Help; using SharpMud.Engine.Sessions; namespace SharpMud.Engine.Tests.Commands; @@ -24,6 +25,17 @@ private static Thing MakeActor(SecurityRole roles) return actor; } + private static HelpCommand MakeSut( + ICommandRegistry? registry = null, + IHelpRepository? helpRepository = null, + IHelpSearchIndex? helpSearchIndex = null) + { + return new HelpCommand( + registry ?? new CommandRegistry(), + helpRepository ?? Substitute.For(), + helpSearchIndex ?? Substitute.For()); + } + [Fact] public async Task ExecuteAsync_OmitsRoleGatedCommand_WhenActorLacksTheRequiredRole() { @@ -33,7 +45,7 @@ public async Task ExecuteAsync_OmitsRoleGatedCommand_WhenActorLacksTheRequiredRo registry.RegisterWithRole(new FakeCommand("ban"), SecurityRole.FullAdmin); var actor = MakeActor(SecurityRole.Player); - var sut = new HelpCommand(registry); + var sut = MakeSut(registry); var ctx = new CommandContext(actor, actor, [], new World(), session); await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); @@ -50,7 +62,7 @@ public async Task ExecuteAsync_IncludesRoleGatedCommand_WhenActorHasTheRequiredR registry.RegisterWithRole(new FakeCommand("ban"), SecurityRole.FullAdmin); var actor = MakeActor(SecurityRole.FullAdmin); - var sut = new HelpCommand(registry); + var sut = MakeSut(registry); var ctx = new CommandContext(actor, actor, [], new World(), session); await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); @@ -66,11 +78,88 @@ public async Task ExecuteAsync_AlwaysIncludesNonGatedCommands_RegardlessOfRole() registry.RegisterOpen(new FakeCommand("look")); var actor = MakeActor(SecurityRole.Player); - var sut = new HelpCommand(registry); + var sut = MakeSut(registry); var ctx = new CommandContext(actor, actor, [], new World(), session); await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); await session.Received(1).WriteLineAsync(Arg.Is(s => s!.Contains("look")), Arg.Any()); } + + [Fact] + public async Task ExecuteAsync_WritesTopicBody_OnExactKeyMatch() + { + var session = Substitute.For(); + var actor = MakeActor(SecurityRole.Player); + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "How to become a wizard." }; + var helpRepository = Substitute.For(); + helpRepository.FindByNameOrAliasAsync("wizard", Arg.Any()).Returns(topic); + + var sut = MakeSut(helpRepository: helpRepository); + var ctx = new CommandContext(actor, actor, ["wizard"], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await session.Received(1).WriteLineAsync("How to become a wizard.", Arg.Any()); + await helpRepository.DidNotReceiveWithAnyArgs().FindByKeywordAsync(default!, default); + } + + [Fact] + public async Task ExecuteAsync_FallsBackToKeywordMatch_WhenExactMatchMisses() + { + var session = Substitute.For(); + var actor = MakeActor(SecurityRole.Player); + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "How to become a wizard." }; + var helpRepository = Substitute.For(); + helpRepository.FindByNameOrAliasAsync("magic", Arg.Any()).Returns((HelpTopic?)null); + helpRepository.FindByKeywordAsync("magic", Arg.Any()).Returns([topic]); + var helpSearchIndex = Substitute.For(); + + var sut = MakeSut(helpRepository: helpRepository, helpSearchIndex: helpSearchIndex); + var ctx = new CommandContext(actor, actor, ["magic"], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await session.Received(1).WriteLineAsync("How to become a wizard.", Arg.Any()); + await helpSearchIndex.DidNotReceiveWithAnyArgs().SearchAsync(default!, default); + } + + [Fact] + public async Task ExecuteAsync_FallsBackToSemanticSearch_WhenExactAndKeywordMatchesMiss() + { + var session = Substitute.For(); + var actor = MakeActor(SecurityRole.Player); + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "How to become a wizard." }; + var helpRepository = Substitute.For(); + helpRepository.FindByNameOrAliasAsync("how do i become a wizard", Arg.Any()).Returns((HelpTopic?)null); + helpRepository.FindByKeywordAsync("how do i become a wizard", Arg.Any()).Returns([]); + var helpSearchIndex = Substitute.For(); + helpSearchIndex.SearchAsync("how do i become a wizard", Arg.Any()).Returns([new HelpSearchHit(topic, 0.4)]); + + var sut = MakeSut(helpRepository: helpRepository, helpSearchIndex: helpSearchIndex); + var ctx = new CommandContext(actor, actor, ["how", "do", "i", "become", "a", "wizard"], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await session.Received(1).WriteLineAsync("How to become a wizard.", Arg.Any()); + } + + [Fact] + public async Task ExecuteAsync_ReportsNoTopicFound_WhenAllThreeTiersMiss() + { + var session = Substitute.For(); + var actor = MakeActor(SecurityRole.Player); + var helpRepository = Substitute.For(); + helpRepository.FindByNameOrAliasAsync("nonsense", Arg.Any()).Returns((HelpTopic?)null); + helpRepository.FindByKeywordAsync("nonsense", Arg.Any()).Returns([]); + var helpSearchIndex = Substitute.For(); + helpSearchIndex.SearchAsync("nonsense", Arg.Any()).Returns([]); + + var sut = MakeSut(helpRepository: helpRepository, helpSearchIndex: helpSearchIndex); + var ctx = new CommandContext(actor, actor, ["nonsense"], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await session.Received(1).WriteLineAsync("No help topic found for 'nonsense'.", Arg.Any()); + } } diff --git a/tests/SharpMud.Engine.Tests/Help/CosineHelpSearchIndexTests.cs b/tests/SharpMud.Engine.Tests/Help/CosineHelpSearchIndexTests.cs new file mode 100644 index 0000000..6a71d9a --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Help/CosineHelpSearchIndexTests.cs @@ -0,0 +1,89 @@ +using SharpMud.Engine.Help; + +namespace SharpMud.Engine.Tests.Help; + +public sealed class CosineHelpSearchIndexTests +{ + private static HelpTopic MakeTopic(string key, params HelpTopicChunk[] chunks) + { + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = key }; + topic.ReplaceChunks(chunks); + return topic; + } + + [Fact] + public async Task SearchAsync_ReturnsTopic_WhenChunkScoreClearsThreshold() + { + var topic = MakeTopic("wizard", new HelpTopicChunk(Guid.NewGuid(), HelpTopicId.New(), 0, "text", [1f, 0f], "model", "hash")); + var repository = Substitute.For(); + repository.GetAllTopicsAsync(Arg.Any()).Returns([topic]); + var embeddingProvider = Substitute.For(); + embeddingProvider.RelevanceThreshold.Returns(0.15); + embeddingProvider.EmbedAsync("query", Arg.Any()).Returns([1f, 0f]); + + var sut = new CosineHelpSearchIndex(repository, embeddingProvider); + + var hits = await sut.SearchAsync("query", TestContext.Current.CancellationToken); + + hits.Should().ContainSingle(); + hits[0].Topic.Should().Be(topic); + hits[0].Score.Should().BeApproximately(1.0, 0.0001); + } + + [Fact] + public async Task SearchAsync_ReturnsEmpty_WhenBestScoreIsBelowThreshold() + { + var topic = MakeTopic("wizard", new HelpTopicChunk(Guid.NewGuid(), HelpTopicId.New(), 0, "text", [1f, 0f], "model", "hash")); + var repository = Substitute.For(); + repository.GetAllTopicsAsync(Arg.Any()).Returns([topic]); + var embeddingProvider = Substitute.For(); + embeddingProvider.RelevanceThreshold.Returns(0.15); + // Orthogonal vector - cosine similarity 0, well below the threshold. + embeddingProvider.EmbedAsync("query", Arg.Any()).Returns([0f, 1f]); + + var sut = new CosineHelpSearchIndex(repository, embeddingProvider); + + var hits = await sut.SearchAsync("query", TestContext.Current.CancellationToken); + + hits.Should().BeEmpty("a weak match must yield no result, not a wrong-but-confident guess"); + } + + [Fact] + public async Task SearchAsync_OrdersHits_ByDescendingScore() + { + var weakTopic = MakeTopic("weak", new HelpTopicChunk(Guid.NewGuid(), HelpTopicId.New(), 0, "text", [0.2f, 0.98f], "model", "hash")); + var strongTopic = MakeTopic("strong", new HelpTopicChunk(Guid.NewGuid(), HelpTopicId.New(), 0, "text", [1f, 0f], "model", "hash")); + var repository = Substitute.For(); + repository.GetAllTopicsAsync(Arg.Any()).Returns([weakTopic, strongTopic]); + var embeddingProvider = Substitute.For(); + embeddingProvider.RelevanceThreshold.Returns(0.15); + embeddingProvider.EmbedAsync("query", Arg.Any()).Returns([1f, 0f]); + + var sut = new CosineHelpSearchIndex(repository, embeddingProvider); + + var hits = await sut.SearchAsync("query", TestContext.Current.CancellationToken); + + hits.Should().HaveCount(2); + hits[0].Topic.Should().Be(strongTopic); + hits[1].Topic.Should().Be(weakTopic); + } + + [Fact] + public async Task SearchAsync_UsesBestChunkPerTopic_NotFirst() + { + var weakChunk = new HelpTopicChunk(Guid.NewGuid(), HelpTopicId.New(), 0, "weak", [0f, 1f], "model", "hash"); + var strongChunk = new HelpTopicChunk(Guid.NewGuid(), HelpTopicId.New(), 1, "strong", [1f, 0f], "model", "hash"); + var topic = MakeTopic("wizard", weakChunk, strongChunk); + var repository = Substitute.For(); + repository.GetAllTopicsAsync(Arg.Any()).Returns([topic]); + var embeddingProvider = Substitute.For(); + embeddingProvider.RelevanceThreshold.Returns(0.15); + embeddingProvider.EmbedAsync("query", Arg.Any()).Returns([1f, 0f]); + + var sut = new CosineHelpSearchIndex(repository, embeddingProvider); + + var hits = await sut.SearchAsync("query", TestContext.Current.CancellationToken); + + hits.Should().ContainSingle().Which.Score.Should().BeApproximately(1.0, 0.0001); + } +} diff --git a/tests/SharpMud.Engine.Tests/Help/HelpTopicChunkerTests.cs b/tests/SharpMud.Engine.Tests/Help/HelpTopicChunkerTests.cs new file mode 100644 index 0000000..f6f3a65 --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Help/HelpTopicChunkerTests.cs @@ -0,0 +1,29 @@ +using SharpMud.Engine.Help; + +namespace SharpMud.Engine.Tests.Help; + +public sealed class HelpTopicChunkerTests +{ + [Fact] + public void Split_ReturnsEmpty_ForEmptyOrWhitespaceBody() + { + HelpTopicChunker.Split("").Should().BeEmpty(); + HelpTopicChunker.Split(" \n ").Should().BeEmpty(); + } + + [Fact] + public void Split_ReturnsSingleChunk_ForSingleParagraph() + { + var chunks = HelpTopicChunker.Split("Wizards cast spells using mana."); + + chunks.Should().ContainSingle().Which.Should().Be("Wizards cast spells using mana."); + } + + [Fact] + public void Split_SplitsOnBlankLines_IntoTrimmedParagraphs() + { + var chunks = HelpTopicChunker.Split("First paragraph.\n\nSecond paragraph.\n\n Third, with padding. "); + + chunks.Should().Equal("First paragraph.", "Second paragraph.", "Third, with padding."); + } +} diff --git a/tests/SharpMud.Engine.Tests/Help/HelpTopicTests.cs b/tests/SharpMud.Engine.Tests/Help/HelpTopicTests.cs new file mode 100644 index 0000000..99e40fc --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Help/HelpTopicTests.cs @@ -0,0 +1,60 @@ +using SharpMud.Engine.Help; + +namespace SharpMud.Engine.Tests.Help; + +public sealed class HelpTopicTests +{ + [Fact] + public void ContentHash_Changes_WhenBodyChanges() + { + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "First version." }; + var firstHash = topic.ContentHash; + + topic.Body = "Second version."; + + topic.ContentHash.Should().NotBe(firstHash); + } + + [Fact] + public void ContentHash_IsStable_ForUnchangedBody() + { + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "Stable text." }; + + topic.ContentHash.Should().Be(topic.ContentHash); + } + + [Fact] + public void SetAliases_ReplacesPreviousAliases_Wholesale() + { + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard" }; + topic.SetAliases(["mage", "sorcerer"]); + + topic.SetAliases(["spellcaster"]); + + topic.Aliases.Should().Equal("spellcaster"); + } + + [Fact] + public void ReplaceChunks_ReplacesPreviousChunks_Wholesale() + { + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard" }; + var firstChunk = new HelpTopicChunk(Guid.NewGuid(), topic.Id, 0, "old", [1f], "model-v1", "hash1"); + topic.ReplaceChunks([firstChunk]); + + var secondChunk = new HelpTopicChunk(Guid.NewGuid(), topic.Id, 0, "new", [2f], "model-v1", "hash2"); + topic.ReplaceChunks([secondChunk]); + + topic.Chunks.Should().ContainSingle().Which.Should().Be(secondChunk); + } + + [Fact] + public void Touch_UpdatesUpdatedAtUtc() + { + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard" }; + var before = topic.UpdatedAtUtc; + + topic.Touch(); + + topic.UpdatedAtUtc.Should().BeOnOrAfter(before); + } +} diff --git a/tests/SharpMud.Engine.Tests/Help/StubEmbeddingProviderTests.cs b/tests/SharpMud.Engine.Tests/Help/StubEmbeddingProviderTests.cs new file mode 100644 index 0000000..b7ecbb3 --- /dev/null +++ b/tests/SharpMud.Engine.Tests/Help/StubEmbeddingProviderTests.cs @@ -0,0 +1,52 @@ +using SharpMud.Engine.Help; + +namespace SharpMud.Engine.Tests.Help; + +public sealed class StubEmbeddingProviderTests +{ + private readonly StubEmbeddingProvider _sut = new(); + + [Fact] + public async Task EmbedAsync_ReturnsIdenticalVectors_ForIdenticalText() + { + var first = await _sut.EmbedAsync("how do I become a wizard", TestContext.Current.CancellationToken); + var second = await _sut.EmbedAsync("how do I become a wizard", TestContext.Current.CancellationToken); + + first.Should().BeEquivalentTo(second, "the stub provider must be deterministic across calls"); + } + + [Fact] + public async Task EmbedAsync_ReturnsIdenticalVectors_IgnoringCase() + { + var lower = await _sut.EmbedAsync("wizard magic", TestContext.Current.CancellationToken); + var upper = await _sut.EmbedAsync("WIZARD MAGIC", TestContext.Current.CancellationToken); + + lower.Should().BeEquivalentTo(upper); + } + + [Fact] + public async Task EmbedAsync_ReturnsDifferentVectors_ForDisjointText() + { + var wizard = await _sut.EmbedAsync("wizard", TestContext.Current.CancellationToken); + var shopkeeper = await _sut.EmbedAsync("shopkeeper commerce", TestContext.Current.CancellationToken); + + wizard.Should().NotBeEquivalentTo(shopkeeper); + } + + [Fact] + public async Task EmbedAsync_ReturnsNormalizedVector() + { + var vector = await _sut.EmbedAsync("wizard magic spell", TestContext.Current.CancellationToken); + + var magnitude = Math.Sqrt(vector.Sum(v => (double)v * v)); + magnitude.Should().BeApproximately(1.0, 0.0001); + } + + [Fact] + public async Task EmbedAsync_ReturnsZeroVector_ForEmptyText() + { + var vector = await _sut.EmbedAsync("", TestContext.Current.CancellationToken); + + vector.Should().OnlyContain(v => v == 0f); + } +} diff --git a/tests/SharpMud.Persistence.Tests/HelpRepositoryTests.cs b/tests/SharpMud.Persistence.Tests/HelpRepositoryTests.cs new file mode 100644 index 0000000..82839e9 --- /dev/null +++ b/tests/SharpMud.Persistence.Tests/HelpRepositoryTests.cs @@ -0,0 +1,117 @@ +using SharpMud.Engine.Help; +using SharpMud.Persistence.Tests.TestKit; + +namespace SharpMud.Persistence.Tests; + +public sealed class HelpRepositoryTests : IDisposable +{ + private readonly TestDbContextFactory _factory = new(); + private readonly HelpRepository _sut; + + public HelpRepositoryTests() + { + _sut = new HelpRepository(_factory); + } + + public void Dispose() => _factory.Dispose(); + + [Fact] + public async Task SaveTopicAsync_ThenGetAllTopicsAsync_RoundTripsTopicWithAliasesKeywordsAndChunks() + { + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Category = "classes", Body = "How to become a wizard." }; + topic.SetAliases(["mage", "sorcerer"]); + topic.SetKeywords(["magic", "spellcasting"]); + topic.ReplaceChunks([ + new HelpTopicChunk(Guid.NewGuid(), topic.Id, 0, "How to become a wizard.", [0.1f, 0.2f, 0.3f], "stub-hashed-bow-v1", topic.ContentHash), + ]); + + await _sut.SaveTopicAsync(topic, TestContext.Current.CancellationToken); + + var all = await _sut.GetAllTopicsAsync(TestContext.Current.CancellationToken); + + var loaded = all.Should().ContainSingle().Subject; + loaded.Key.Should().Be("wizard"); + loaded.Category.Should().Be("classes"); + loaded.Body.Should().Be("How to become a wizard."); + loaded.Aliases.Should().BeEquivalentTo(["mage", "sorcerer"]); + loaded.Keywords.Should().BeEquivalentTo(["magic", "spellcasting"]); + + var chunk = loaded.Chunks.Should().ContainSingle().Subject; + chunk.Text.Should().Be("How to become a wizard."); + chunk.Embedding.Should().BeEquivalentTo(new[] { 0.1f, 0.2f, 0.3f }); + chunk.EmbeddingModelId.Should().Be("stub-hashed-bow-v1"); + } + + [Fact] + public async Task FindByNameOrAliasAsync_MatchesByKey_CaseInsensitive() + { + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "Text." }; + await _sut.SaveTopicAsync(topic, TestContext.Current.CancellationToken); + + var found = await _sut.FindByNameOrAliasAsync("WIZARD", TestContext.Current.CancellationToken); + + found.Should().NotBeNull(); + found!.Id.Should().Be(topic.Id); + } + + [Fact] + public async Task FindByNameOrAliasAsync_MatchesByAlias() + { + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "Text." }; + topic.SetAliases(["mage"]); + await _sut.SaveTopicAsync(topic, TestContext.Current.CancellationToken); + + var found = await _sut.FindByNameOrAliasAsync("mage", TestContext.Current.CancellationToken); + + found.Should().NotBeNull(); + found!.Id.Should().Be(topic.Id); + } + + [Fact] + public async Task FindByNameOrAliasAsync_ReturnsNull_WhenNoMatch() + { + var found = await _sut.FindByNameOrAliasAsync("nobody", TestContext.Current.CancellationToken); + + found.Should().BeNull(); + } + + [Fact] + public async Task FindByKeywordAsync_ReturnsMatchingTopics() + { + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "Text." }; + topic.SetKeywords(["magic"]); + await _sut.SaveTopicAsync(topic, TestContext.Current.CancellationToken); + + var found = await _sut.FindByKeywordAsync("magic", TestContext.Current.CancellationToken); + + found.Should().ContainSingle().Which.Id.Should().Be(topic.Id); + } + + [Fact] + public async Task SaveTopicAsync_CalledTwice_ReplacesChunksWholesale() + { + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "Text." }; + topic.ReplaceChunks([new HelpTopicChunk(Guid.NewGuid(), topic.Id, 0, "old", [1f], "model-v1", "hash1")]); + await _sut.SaveTopicAsync(topic, TestContext.Current.CancellationToken); + + topic.ReplaceChunks([new HelpTopicChunk(Guid.NewGuid(), topic.Id, 0, "new", [2f], "model-v2", "hash2")]); + await _sut.SaveTopicAsync(topic, TestContext.Current.CancellationToken); + + var all = await _sut.GetAllTopicsAsync(TestContext.Current.CancellationToken); + var loaded = all.Should().ContainSingle().Subject; + loaded.Chunks.Should().ContainSingle().Which.Text.Should().Be("new"); + } + + [Fact] + public async Task DeleteTopicAsync_RemovesTopicAndItsChunks() + { + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "Text." }; + topic.ReplaceChunks([new HelpTopicChunk(Guid.NewGuid(), topic.Id, 0, "text", [1f], "model", "hash")]); + await _sut.SaveTopicAsync(topic, TestContext.Current.CancellationToken); + + await _sut.DeleteTopicAsync(topic.Id, TestContext.Current.CancellationToken); + + var all = await _sut.GetAllTopicsAsync(TestContext.Current.CancellationToken); + all.Should().BeEmpty(); + } +} diff --git a/tests/SharpMud.Ruleset.Rpg.Tests/ServiceCollectionExtensionsTests.cs b/tests/SharpMud.Ruleset.Rpg.Tests/ServiceCollectionExtensionsTests.cs index a9d2ad3..c3983e5 100644 --- a/tests/SharpMud.Ruleset.Rpg.Tests/ServiceCollectionExtensionsTests.cs +++ b/tests/SharpMud.Ruleset.Rpg.Tests/ServiceCollectionExtensionsTests.cs @@ -1,6 +1,7 @@ using Microsoft.Extensions.DependencyInjection; using SharpMud.Engine.Commands; using SharpMud.Engine.Core; +using SharpMud.Engine.Help; using SharpMud.Engine.Ticking; namespace SharpMud.Ruleset.Rpg.Tests; @@ -17,6 +18,8 @@ public void AddSharpMudRpgRuleset_ComposesBuiltinRpgAndConsumerCommands_IntoOneR { var services = new ServiceCollection(); services.AddSingleton(Substitute.For()); + services.AddSingleton(Substitute.For()); + services.AddSingleton(Substitute.For()); services.AddSharpMudRpgRuleset((_, registry) => registry.RegisterOpen(new FakeConsumerCommand())); @@ -37,6 +40,8 @@ public void AddSharpMudRpgRuleset_RegistersCombatManagerAsBothItselfAndTickable_ { var services = new ServiceCollection(); services.AddSingleton(Substitute.For()); + services.AddSingleton(Substitute.For()); + services.AddSingleton(Substitute.For()); services.AddSharpMudRpgRuleset(); From 944ada4780e73b178cde4f794900c99ea32c6510 Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Fri, 24 Jul 2026 12:04:12 -0400 Subject: [PATCH 2/3] fix(engine): address PR #21 review feedback on help system - HelpTopicEditCommand no longer reassigns HelpTopic.Key on an edit - only set on the create-new-topic path. The unconditional reassignment let helptopic Wizard silently rename a wizard topic's stored casing, and would rename a topic's canonical Key to alias text once aliases become settable. - HelpCommand's keyword tier now checks each word of a multi-word query against Keywords individually, since FindByKeywordAsync's contract is an exact match against a single entry - the whole query never matched before. - HelpRepository.FindByKeywordAsync now orders results by Key so a caller taking FirstOrDefault when multiple topics share a keyword gets a deterministic result, not an arbitrary one. - HelpTopicChunkConfiguration now configures a real FK (HelpTopicId -> HelpTopic.Id, cascade delete, no navigation property) - the doc comment already claimed this but nothing enforced it at the DB level. - HelpTopicConfiguration adds a unique index on Key, closing a check-then-act race in HelpTopicEditCommand's create path. - Documented (PLAN-0010, docs/help-system.md) that HelpTopicChunker.Split's paragraph-boundary chunking is unreachable via the v1 helptopic command, since single-line Telnet input collapses every blank line before Body is ever set - deliberately not fixed here, needs its own design pass. 5 new regression tests covering the four code fixes. --- docs/help-system.md | 7 +++ .../plans/0010-help-system-semantic-search.md | 15 +++++ .../Builtin/Admin/HelpTopicEditCommand.cs | 17 +++++- .../Commands/Builtin/HelpCommand.cs | 14 ++++- .../HelpTopicChunkConfiguration.cs | 15 +++++ .../Configurations/HelpTopicConfiguration.cs | 10 ++++ src/SharpMud.Persistence/HelpRepository.cs | 6 ++ .../Admin/HelpTopicEditCommandTests.cs | 21 +++++++ .../Commands/HelpCommandTests.cs | 26 ++++++++ .../HelpRepositoryTests.cs | 59 +++++++++++++++++++ 10 files changed, 185 insertions(+), 5 deletions(-) diff --git a/docs/help-system.md b/docs/help-system.md index 5b8631c..0b5b313 100644 --- a/docs/help-system.md +++ b/docs/help-system.md @@ -164,3 +164,10 @@ succeed. semantic matching needs to opt in the same way (or write their own `IEmbeddingProvider`/`RelevanceThreshold`). - No DynamoDB mapping for `IHelpRepository` yet. +- `HelpTopicChunker.Split`'s paragraph-boundary (blank-line) chunking is + unreachable via the v1 authoring command — `helptopic ` + reconstructs `Body` from single-line Telnet input, which has already + collapsed every whitespace run (including any newline) to a single + space before the command ever sees it. Every real topic chunks to + exactly 1 piece today, regardless of length. Found in PR review; see + PLAN-0010's Open questions for the fix options considered. diff --git a/docs/plans/0010-help-system-semantic-search.md b/docs/plans/0010-help-system-semantic-search.md index a63da2d..bf92873 100644 --- a/docs/plans/0010-help-system-semantic-search.md +++ b/docs/plans/0010-help-system-semantic-search.md @@ -191,3 +191,18 @@ grant `FullAdmin`, then `rolegrant Adventurer minorbuilder` to reach `CommandParser`'s no-quoted-strings constraint per ADR-0009) is needed before the keyword-lookup tier has any real content to match against in practice. +- **`HelpTopicChunker.Split`'s paragraph-boundary (blank-line) chunking is + unreachable given the v1 authoring command** — found in PR review. + `helptopic ` reconstructs `Body` via + `string.Join(' ', ctx.Args.Skip(1))`, and `CommandParser`'s single-line + Telnet input has already collapsed every whitespace run (including any + newline) to a single space before `ctx.Args` ever sees it. There is + currently no way to author a body containing a blank line, so every real + topic chunks to exactly 1 piece today regardless of length — ADR-0010's + paragraph-level chunking design doesn't do anything yet in practice. + Deliberately not fixed in this slice: a real fix is either a multi-line + input mechanism (no such thing exists over single-line Telnet per + ADR-0009) or a different chunking fallback (e.g. sentence- or + fixed-width-window splitting when no blank lines are found), and both are + their own design decision, not a one-line fix. Revisit once multi-chunk + topics actually matter for semantic-search precision. diff --git a/src/SharpMud.Engine/Commands/Builtin/Admin/HelpTopicEditCommand.cs b/src/SharpMud.Engine/Commands/Builtin/Admin/HelpTopicEditCommand.cs index 6d7166b..fda4d49 100644 --- a/src/SharpMud.Engine/Commands/Builtin/Admin/HelpTopicEditCommand.cs +++ b/src/SharpMud.Engine/Commands/Builtin/Admin/HelpTopicEditCommand.cs @@ -6,8 +6,12 @@ namespace SharpMud.Engine.Commands.Builtin.Admin; /// The helptopic <key> <body> command (, ADR-0010) - creates a new /// or overwrites an existing one's , keyed by exact, case-insensitive match. This is the only authoring path for help +/// cref="HelpTopic.Body"/>. An existing topic is found the same way +/// lookup works ( - +/// exact, case-insensitive match against or +/// an alias), but only is ever overwritten - +/// is only set on the create-new-topic path, +/// never reassigned on an edit. This is the only authoring path for help /// content - no file-based alternative (see ADR-0010's Decision Outcome). /// Aliases/keywords aren't settable via this command yet - deliberately out /// of v1 scope, see PLAN-0010's Open questions. @@ -40,8 +44,15 @@ public async Task ExecuteAsync(CommandContext ctx, CancellationToken ct) var key = ctx.Args[0]; var body = string.Join(' ', ctx.Args.Skip(1)); + // Key is only ever set here on the create-new-topic path (via the + // object initializer below) - an existing topic keeps its + // canonical Key even though the lookup that found it is + // case-insensitive and (eventually) alias-aware. Reassigning Key + // unconditionally here would let `helptopic Wizard ...` silently + // rename a `wizard` topic's stored casing today, and once aliases + // are settable, editing by an alias would rename the canonical Key + // to the alias text - caught in PR review. var topic = await _repository.FindByNameOrAliasAsync(key, ct) ?? new HelpTopic { Id = HelpTopicId.New(), Key = key }; - topic.Key = key; topic.Body = body; topic.Touch(); diff --git a/src/SharpMud.Engine/Commands/Builtin/HelpCommand.cs b/src/SharpMud.Engine/Commands/Builtin/HelpCommand.cs index 28f5660..a7b7e4a 100644 --- a/src/SharpMud.Engine/Commands/Builtin/HelpCommand.cs +++ b/src/SharpMud.Engine/Commands/Builtin/HelpCommand.cs @@ -52,8 +52,18 @@ private async Task ExecuteTopicLookupAsync(CommandContext ctx, CancellationToken if (topic is null) { - var keywordMatches = await _helpRepository.FindByKeywordAsync(query, ct); - topic = keywordMatches.FirstOrDefault(); + // FindByKeywordAsync's contract is an exact match against one + // Keywords entry, not a substring/full-phrase match - so a + // multi-word query has to be checked one token at a time, or a + // query like "teach me magic" could never hit a "magic" + // keyword. First token to match wins - caught in PR review. + foreach (var token in query.Split(' ', StringSplitOptions.RemoveEmptyEntries)) + { + var keywordMatches = await _helpRepository.FindByKeywordAsync(token, ct); + topic = keywordMatches.FirstOrDefault(); + if (topic is not null) + break; + } } if (topic is null) diff --git a/src/SharpMud.Persistence/Configurations/HelpTopicChunkConfiguration.cs b/src/SharpMud.Persistence/Configurations/HelpTopicChunkConfiguration.cs index 0ace950..a2029e8 100644 --- a/src/SharpMud.Persistence/Configurations/HelpTopicChunkConfiguration.cs +++ b/src/SharpMud.Persistence/Configurations/HelpTopicChunkConfiguration.cs @@ -18,6 +18,21 @@ public void Configure(EntityTypeBuilder builder) builder.HasKey(x => x.Id); builder.Property(x => x.HelpTopicId).HasConversion(id => id.Value, value => new HelpTopicId(value)); builder.HasIndex(x => x.HelpTopicId); + + // A real FK constraint (no navigation property needed on either + // side) - HelpRepository.SaveTopicAsync/DeleteTopicAsync already + // delete/insert a topic and its chunks in the same + // SaveChangesAsync batch, so EF's automatic dependency ordering + // doesn't change either path's behavior. This makes the "explicit + // HelpTopicId FK" claim in the comment above actually true at the + // DB level, and cascades a delete so nothing outside + // HelpRepository can orphan a topic's chunks - caught in PR + // review. + builder.HasOne() + .WithMany() + .HasForeignKey(x => x.HelpTopicId) + .OnDelete(DeleteBehavior.Cascade); + builder.Property(x => x.Text).IsRequired(); builder.Property(x => x.EmbeddingModelId).IsRequired(); builder.Property(x => x.SourceContentHash).IsRequired(); diff --git a/src/SharpMud.Persistence/Configurations/HelpTopicConfiguration.cs b/src/SharpMud.Persistence/Configurations/HelpTopicConfiguration.cs index 29c6a87..559199e 100644 --- a/src/SharpMud.Persistence/Configurations/HelpTopicConfiguration.cs +++ b/src/SharpMud.Persistence/Configurations/HelpTopicConfiguration.cs @@ -20,6 +20,16 @@ public void Configure(EntityTypeBuilder builder) builder.HasKey(x => x.Id); builder.Property(x => x.Id).HasConversion(id => id.Value, value => new HelpTopicId(value)); builder.Property(x => x.Key).IsRequired(); + + // Guards the check-then-act gap in HelpTopicEditCommand + // (FindByNameOrAliasAsync -> create-if-null -> SaveTopicAsync, no + // transaction spanning it) - without this, two concurrent + // `helptopic newtopic ...` calls for the same new key could both + // observe null and each insert a row, leaving FindByNameOrAliasAsync's + // FirstOrDefault to pick one arbitrarily. Low-probability today + // (solo/small-collaborator project), but cheap - caught in PR + // review. + builder.HasIndex(x => x.Key).IsUnique(); builder.Property(x => x.Category).IsRequired(); builder.Property(x => x.Body).IsRequired(); builder.Property(x => x.UpdatedAtUtc).IsRequired(); diff --git a/src/SharpMud.Persistence/HelpRepository.cs b/src/SharpMud.Persistence/HelpRepository.cs index dc9fe37..84bba2a 100644 --- a/src/SharpMud.Persistence/HelpRepository.cs +++ b/src/SharpMud.Persistence/HelpRepository.cs @@ -24,6 +24,12 @@ public async Task> FindByKeywordAsync(string keyword, C var topics = await GetAllTopicsAsync(ct); return topics .Where(t => t.Keywords.Any(k => string.Equals(k, keyword, StringComparison.OrdinalIgnoreCase))) + // Deterministic order (not insertion/rowid order, which SQL + // makes no guarantee about) so a caller taking FirstOrDefault + // when multiple topics share a keyword gets a consistent + // result across calls, not an arbitrary one - caught in PR + // review. + .OrderBy(t => t.Key, StringComparer.OrdinalIgnoreCase) .ToList(); } diff --git a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/HelpTopicEditCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/HelpTopicEditCommandTests.cs index 0334000..7e73950 100644 --- a/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/HelpTopicEditCommandTests.cs +++ b/tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/HelpTopicEditCommandTests.cs @@ -52,6 +52,27 @@ await repository.Received(1).SaveTopicAsync( Arg.Any()); } + [Fact] + public async Task ExecuteAsync_DoesNotReassignKey_WhenExistingTopicFoundByDifferentCasing() + { + var (actor, world, session) = MakeActor(); + var existing = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "Old text." }; + var repository = Substitute.For(); + // FindByNameOrAliasAsync is case-insensitive - "Wizard" finds the + // "wizard" topic, same as it would for an alias once those are + // settable. + repository.FindByNameOrAliasAsync("Wizard", Arg.Any()).Returns(existing); + + var sut = new HelpTopicEditCommand(repository); + var ctx = new CommandContext(actor, actor, ["Wizard", "New", "text."], world, session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await repository.Received(1).SaveTopicAsync( + Arg.Is(t => t.Key == "wizard" && t.Body == "New text."), + Arg.Any()); + } + [Fact] public async Task ExecuteAsync_SendsUsageMessage_WhenMissingBody() { diff --git a/tests/SharpMud.Engine.Tests/Commands/HelpCommandTests.cs b/tests/SharpMud.Engine.Tests/Commands/HelpCommandTests.cs index 4e3067a..1154253 100644 --- a/tests/SharpMud.Engine.Tests/Commands/HelpCommandTests.cs +++ b/tests/SharpMud.Engine.Tests/Commands/HelpCommandTests.cs @@ -124,6 +124,32 @@ public async Task ExecuteAsync_FallsBackToKeywordMatch_WhenExactMatchMisses() await helpSearchIndex.DidNotReceiveWithAnyArgs().SearchAsync(default!, default); } + [Fact] + public async Task ExecuteAsync_ChecksEachQueryWordAgainstKeywords_ForMultiWordQueries() + { + var session = Substitute.For(); + var actor = MakeActor(SecurityRole.Player); + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "How to become a wizard." }; + var helpRepository = Substitute.For(); + var query = "teach me magic"; + helpRepository.FindByNameOrAliasAsync(query, Arg.Any()).Returns((HelpTopic?)null); + // FindByKeywordAsync's contract is an exact match against one + // Keywords entry - the query as a whole ("teach me magic") never + // matches, only the individual token "magic" does. + helpRepository.FindByKeywordAsync("teach", Arg.Any()).Returns([]); + helpRepository.FindByKeywordAsync("me", Arg.Any()).Returns([]); + helpRepository.FindByKeywordAsync("magic", Arg.Any()).Returns([topic]); + var helpSearchIndex = Substitute.For(); + + var sut = MakeSut(helpRepository: helpRepository, helpSearchIndex: helpSearchIndex); + var ctx = new CommandContext(actor, actor, ["teach", "me", "magic"], new World(), session); + + await sut.ExecuteAsync(ctx, TestContext.Current.CancellationToken); + + await session.Received(1).WriteLineAsync("How to become a wizard.", Arg.Any()); + await helpSearchIndex.DidNotReceiveWithAnyArgs().SearchAsync(default!, default); + } + [Fact] public async Task ExecuteAsync_FallsBackToSemanticSearch_WhenExactAndKeywordMatchesMiss() { diff --git a/tests/SharpMud.Persistence.Tests/HelpRepositoryTests.cs b/tests/SharpMud.Persistence.Tests/HelpRepositoryTests.cs index 82839e9..9da51a8 100644 --- a/tests/SharpMud.Persistence.Tests/HelpRepositoryTests.cs +++ b/tests/SharpMud.Persistence.Tests/HelpRepositoryTests.cs @@ -1,3 +1,4 @@ +using Microsoft.EntityFrameworkCore; using SharpMud.Engine.Help; using SharpMud.Persistence.Tests.TestKit; @@ -87,6 +88,64 @@ public async Task FindByKeywordAsync_ReturnsMatchingTopics() found.Should().ContainSingle().Which.Id.Should().Be(topic.Id); } + [Fact] + public async Task FindByKeywordAsync_OrdersMultipleMatches_ByKey() + { + var zebra = new HelpTopic { Id = HelpTopicId.New(), Key = "zebra-topic", Body = "Text." }; + zebra.SetKeywords(["magic"]); + var alpha = new HelpTopic { Id = HelpTopicId.New(), Key = "alpha-topic", Body = "Text." }; + alpha.SetKeywords(["magic"]); + await _sut.SaveTopicAsync(zebra, TestContext.Current.CancellationToken); + await _sut.SaveTopicAsync(alpha, TestContext.Current.CancellationToken); + + var found = await _sut.FindByKeywordAsync("magic", TestContext.Current.CancellationToken); + + found.Select(t => t.Key).Should().Equal("alpha-topic", "zebra-topic"); + } + + [Fact] + public async Task SaveTopicAsync_ThrowsOnDuplicateKey_WhenBypassingRepositorysOwnIdDedupe() + { + var first = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "Text." }; + await _sut.SaveTopicAsync(first, TestContext.Current.CancellationToken); + + // A distinct Id but the same Key - HelpRepository.SaveTopicAsync + // only dedupes by Id, so this simulates two concurrent creates for + // the same new key racing each other; the unique index is what + // actually prevents the duplicate row. + var second = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "Other text." }; + + var act = async () => await _sut.SaveTopicAsync(second, TestContext.Current.CancellationToken); + + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task DeletingATopicDirectly_CascadesToItsChunks_EvenBypassingHelpRepository() + { + var topic = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "Text." }; + topic.ReplaceChunks([new HelpTopicChunk(Guid.NewGuid(), topic.Id, 0, "text", [1f], "model", "hash")]); + await _sut.SaveTopicAsync(topic, TestContext.Current.CancellationToken); + + // Deletes the HelpTopic row directly, bypassing + // HelpRepository.DeleteTopicAsync entirely (which would clean up + // chunks itself) - proves the FK's cascade delete is what actually + // prevents an orphaned chunk here, not just HelpRepository's own + // discipline. + await using (var context = await _factory.CreateDbContextAsync(TestContext.Current.CancellationToken)) + { + var row = await context.HelpTopics.SingleAsync(t => t.Id == topic.Id, TestContext.Current.CancellationToken); + context.HelpTopics.Remove(row); + await context.SaveChangesAsync(TestContext.Current.CancellationToken); + } + + await using (var context = await _factory.CreateDbContextAsync(TestContext.Current.CancellationToken)) + { + var orphanedChunks = await context.HelpTopicChunks.Where(c => c.HelpTopicId == topic.Id).ToListAsync(TestContext.Current.CancellationToken); + orphanedChunks.Should().BeEmpty(); + } + } + [Fact] public async Task SaveTopicAsync_CalledTwice_ReplacesChunksWholesale() { From 984c1cab80fada7f75ef4ccc676865b0247a92d9 Mon Sep 17 00:00:00 2001 From: Nick Cipollina Date: Fri, 24 Jul 2026 12:19:20 -0400 Subject: [PATCH 3/3] fix(engine): close case-insensitive gap in help topic unique index The unique index on HelpTopic.Key added in the previous review round didn't actually close the race it was meant to fix - SQLite's default collation is case-sensitive, but HelpRepository's lookups (FindByNameOrAliasAsync) match Key case-insensitively. "wizard" and "Wizard" passed as two distinct rows at the DB level despite the app treating them as the same topic. Added .UseCollation("NOCASE") to Key so the index enforces uniqueness under the same semantics the app already uses. Also documents (PLAN-0010, docs/help-system.md) that HelpCommand's multi-word keyword-tier loop reloads the full topic corpus once per query word - non-blocking per review, deliberately not optimized here. 1 new regression test. --- docs/help-system.md | 5 +++++ .../plans/0010-help-system-semantic-search.md | 13 ++++++++++++ .../Configurations/HelpTopicConfiguration.cs | 10 +++++++++- .../HelpRepositoryTests.cs | 20 +++++++++++++++++++ 4 files changed, 47 insertions(+), 1 deletion(-) diff --git a/docs/help-system.md b/docs/help-system.md index 0b5b313..91af0de 100644 --- a/docs/help-system.md +++ b/docs/help-system.md @@ -171,3 +171,8 @@ succeed. space before the command ever sees it. Every real topic chunks to exactly 1 piece today, regardless of length. Found in PR review; see PLAN-0010's Open questions for the fix options considered. +- `HelpCommand`'s keyword tier reloads the entire topic corpus once per + query word (`FindByKeywordAsync` calls `GetAllTopicsAsync` internally), + since multi-word tokenization was added in PR review. Fine at today's + scale, worth optimizing (load once, filter tokens in memory) if it ever + becomes a hot path — see PLAN-0010's Open questions. diff --git a/docs/plans/0010-help-system-semantic-search.md b/docs/plans/0010-help-system-semantic-search.md index bf92873..11f3e05 100644 --- a/docs/plans/0010-help-system-semantic-search.md +++ b/docs/plans/0010-help-system-semantic-search.md @@ -206,3 +206,16 @@ grant `FullAdmin`, then `rolegrant Adventurer minorbuilder` to reach fixed-width-window splitting when no blank lines are found), and both are their own design decision, not a one-line fix. Revisit once multi-chunk topics actually matter for semantic-search precision. +- **`HelpCommand`'s per-token keyword-tier loop reloads the entire topic + corpus once per query word** — found in a second round of PR review, on + the multi-word tokenization fix above. `FindByKeywordAsync` calls + `HelpRepository.GetAllTopicsAsync` (a full corpus load via a fresh + `DbContext`) internally, so a 5-word query does 5 full-corpus round + trips instead of 1. Fine at today's help-topic scale — the same "load + everything" shape already accepted elsewhere in this repo (see + `ThingRepository`, and ADR-0010's own Negative Consequences) — and the + reviewer flagged it as a suggestion, not a blocker. Deliberately not + fixed here; the real fix (load the corpus once, check every token + against it in memory - e.g. an `IHelpRepository` method taking multiple + keywords, or hoisting `GetAllTopicsAsync` up into `HelpCommand`) is worth + doing if/when this becomes an actual hot path, not preemptively. diff --git a/src/SharpMud.Persistence/Configurations/HelpTopicConfiguration.cs b/src/SharpMud.Persistence/Configurations/HelpTopicConfiguration.cs index 559199e..def5da7 100644 --- a/src/SharpMud.Persistence/Configurations/HelpTopicConfiguration.cs +++ b/src/SharpMud.Persistence/Configurations/HelpTopicConfiguration.cs @@ -19,7 +19,15 @@ public void Configure(EntityTypeBuilder builder) builder.ToTable("HelpTopics"); builder.HasKey(x => x.Id); builder.Property(x => x.Id).HasConversion(id => id.Value, value => new HelpTopicId(value)); - builder.Property(x => x.Key).IsRequired(); + // NOCASE, not SQLite's default BINARY collation - HelpRepository's + // app-level lookups (FindByNameOrAliasAsync) already match Key + // case-insensitively, so the unique index below has to enforce + // uniqueness under that same semantics or it doesn't actually + // guard anything: without this, "wizard" and "Wizard" pass as two + // distinct rows at the DB level even though the app treats them as + // the same topic - caught in PR review (the index alone, added in + // the previous round, didn't close the race it was meant to). + builder.Property(x => x.Key).IsRequired().UseCollation("NOCASE"); // Guards the check-then-act gap in HelpTopicEditCommand // (FindByNameOrAliasAsync -> create-if-null -> SaveTopicAsync, no diff --git a/tests/SharpMud.Persistence.Tests/HelpRepositoryTests.cs b/tests/SharpMud.Persistence.Tests/HelpRepositoryTests.cs index 9da51a8..67168c7 100644 --- a/tests/SharpMud.Persistence.Tests/HelpRepositoryTests.cs +++ b/tests/SharpMud.Persistence.Tests/HelpRepositoryTests.cs @@ -120,6 +120,26 @@ public async Task SaveTopicAsync_ThrowsOnDuplicateKey_WhenBypassingRepositorysOw await act.Should().ThrowAsync(); } + [Fact] + public async Task SaveTopicAsync_ThrowsOnDuplicateKey_EvenWhenCasingDiffers() + { + // The unique index alone (added in an earlier PR-review round) + // didn't close this race - SQLite's default collation is + // case-sensitive, but HelpRepository's own lookups + // (FindByNameOrAliasAsync) match Key case-insensitively. Without + // the NOCASE collation on Key, "wizard" and "Wizard" would pass as + // two distinct rows even though the app treats them as the same + // topic - caught in a second round of PR review. + var first = new HelpTopic { Id = HelpTopicId.New(), Key = "wizard", Body = "Text." }; + await _sut.SaveTopicAsync(first, TestContext.Current.CancellationToken); + + var second = new HelpTopic { Id = HelpTopicId.New(), Key = "Wizard", Body = "Other text." }; + + var act = async () => await _sut.SaveTopicAsync(second, TestContext.Current.CancellationToken); + + await act.Should().ThrowAsync(); + } + [Fact] public async Task DeletingATopicDirectly_CascadesToItsChunks_EvenBypassingHelpRepository() {