feat(engine): help system with keyword/semantic search fallback#21
Merged
Conversation
Slice 5 of the WheelMUD reconciliation roadmap. `help <query>` 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.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
Implements a new persisted in-game help-topic system in SharpMud.Engine, extending help <query> from command listing into a three-tier lookup pipeline (exact key/alias → keyword → semantic-search fallback) with SQLite persistence and a swappable embedding/search abstraction; also updates docs and branding, and adds a sample-only local embedding provider.
Changes:
- Adds
HelpTopic/HelpTopicChunkdomain model, repository/search abstractions, and a default brute-force cosine search index with a deterministic stub embedding provider. - Extends
helpto resolve topics via the three-tier pipeline and adds role-gated admin commands for authoring topics and rebuilding the semantic index. - Wires help persistence/search into the SQLite DI registration and sample host, and updates docs/docsite navigation plus branding assets.
Reviewed changes
Copilot reviewed 50 out of 52 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/SharpMud.Ruleset.Rpg.Tests/ServiceCollectionExtensionsTests.cs | Updates ruleset DI composition tests for new help-related dependencies. |
| tests/SharpMud.Persistence.Tests/HelpRepositoryTests.cs | Adds persistence tests for help topic/chunk round-tripping and lookup behavior. |
| tests/SharpMud.Engine.Tests/Help/StubEmbeddingProviderTests.cs | Adds determinism/normalization tests for the stub embedding provider. |
| tests/SharpMud.Engine.Tests/Help/HelpTopicTests.cs | Adds unit tests for help topic domain behaviors (hashing, alias/chunk replacement, timestamps). |
| tests/SharpMud.Engine.Tests/Help/HelpTopicChunkerTests.cs | Adds tests for paragraph chunking logic used during index rebuild. |
| tests/SharpMud.Engine.Tests/Help/CosineHelpSearchIndexTests.cs | Adds tests for thresholding, ordering, and best-chunk selection in semantic search. |
| tests/SharpMud.Engine.Tests/Commands/HelpCommandTests.cs | Extends help command tests to cover the three-tier lookup pipeline. |
| tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/HelpTopicEditCommandTests.cs | Adds tests for the helptopic admin command. |
| tests/SharpMud.Engine.Tests/Commands/Builtin/Admin/HelpIndexRebuildCommandTests.cs | Adds tests for the helpindex rebuild admin command. |
| src/SharpMud.Persistence/HelpRepository.cs | Implements SQLite-backed IHelpRepository with full-corpus load and wholesale chunk replace. |
| src/SharpMud.Persistence/GameDbContext.cs | Registers EF DbSets for help topics and chunks. |
| src/SharpMud.Persistence/Configurations/HelpTopicConfiguration.cs | Adds EF mapping for HelpTopic, including aliases/keywords conversions. |
| src/SharpMud.Persistence/Configurations/HelpTopicChunkConfiguration.cs | Adds EF mapping for HelpTopicChunk, including BLOB embedding serialization. |
| src/SharpMud.Persistence.Sqlite/ServiceCollectionExtensions.cs | Registers help repository/search/index defaults in SQLite persistence DI wiring. |
| src/SharpMud.Hosting/ServiceCollectionExtensions.cs | Updates ruleset composition to pass help services into builtin command registration. |
| src/SharpMud.Engine/Help/StubEmbeddingProvider.cs | Adds deterministic hashed bag-of-words embedding provider + threshold. |
| src/SharpMud.Engine/Help/IHelpSearchIndex.cs | Introduces semantic search abstraction for tier-three help lookup. |
| src/SharpMud.Engine/Help/IHelpRepository.cs | Introduces repository abstraction for persisted help topics and chunks. |
| src/SharpMud.Engine/Help/IEmbeddingProvider.cs | Introduces embedding abstraction and provider-owned relevance threshold. |
| src/SharpMud.Engine/Help/HelpTopicId.cs | Adds a strongly-typed topic identifier. |
| src/SharpMud.Engine/Help/HelpTopicChunker.cs | Adds paragraph-based chunk splitting for index rebuild. |
| src/SharpMud.Engine/Help/HelpTopicChunk.cs | Adds chunk value object carrying text, embedding, and staleness metadata. |
| src/SharpMud.Engine/Help/HelpTopic.cs | Adds help topic aggregate root with aliases/keywords/chunks and content hashing. |
| src/SharpMud.Engine/Help/HelpSearchHit.cs | Adds semantic search hit model (topic + score). |
| src/SharpMud.Engine/Help/HelpContentHashing.cs | Adds shared SHA-256 hashing for topic body staleness detection. |
| src/SharpMud.Engine/Help/CosineHelpSearchIndex.cs | Implements brute-force cosine similarity semantic search across persisted chunks. |
| src/SharpMud.Engine/Commands/Builtin/HelpCommand.cs | Extends help to resolve help topics via exact/keyword/semantic fallback. |
| src/SharpMud.Engine/Commands/Builtin/BuiltinCommands.cs | Updates builtin registration to inject help dependencies into HelpCommand. |
| src/SharpMud.Engine/Commands/Builtin/Admin/HelpTopicEditCommand.cs | Adds helptopic <key> <body> authoring command (role-gated by registrars). |
| src/SharpMud.Engine/Commands/Builtin/Admin/HelpIndexRebuildCommand.cs | Adds helpindex rebuild to regenerate chunk embeddings from topic bodies. |
| src/SharpMud.Engine/Commands/Builtin/Admin/HelpAdminCommands.cs | Adds opt-in registration helper for help authoring commands. |
| samples/SharpMud.Samples.Classic/SharpMud.Samples.Classic.csproj | Adds sample-only dependency on SmartComponents.LocalEmbeddings. |
| samples/SharpMud.Samples.Classic/Program.cs | Overrides the default embedding provider and wires help admin commands in the sample composition root. |
| samples/SharpMud.Samples.Classic/LocalEmbeddingProvider.cs | Adds a local ONNX-backed embedding provider for the sample host. |
| README.md | Updates project branding with icon + ASCII banner. |
| docsite/zensical.toml | Adds docsite navigation entry for the help system page. |
| docsite/docs/index.md | Updates docsite landing page branding with ASCII banner. |
| docsite/docs/help-system.md | Adds public-facing help system documentation (usage + embedding provider swap). |
| docsite/docs/customizing.md | Documents how to override IEmbeddingProvider for semantic help search. |
| docs/research/wheelmud-findings.md | Records WheelMUD help system findings and adoption/deviation decisions. |
| docs/README.md | Indexes new internal help system documentation. |
| docs/plans/README.md | Adds PLAN-0010 entry. |
| docs/plans/0010-help-system-semantic-search.md | Adds execution plan documenting scope, tasks, and verification. |
| docs/plans/0001-wheelmud-reconciliation-roadmap.md | Marks Slice 5 (help system) as done and updates timestamps. |
| docs/help-system.md | Adds internal/engineering help system documentation. |
| docs/commands.md | Updates command documentation for the new help system + authoring commands. |
| docs/adr/README.md | Adds ADR index entries for ADR-0010 and ADR-0011. |
| docs/adr/0011-local-embedding-provider-for-sample.md | Adds ADR for the sample-only local embedding provider and threshold calibration. |
| docs/adr/0010-help-system-semantic-search.md | Adds ADR defining the help topic model, pipeline, persistence, and index rebuild strategy. |
| Directory.Packages.props | Pins SmartComponents.LocalEmbeddings version for the sample project. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ncipollina
commented
Jul 24, 2026
ncipollina
commented
Jul 24, 2026
ncipollina
commented
Jul 24, 2026
ncipollina
commented
Jul 24, 2026
ncipollina
commented
Jul 24, 2026
ncipollina
left a comment
ncipollina
left a comment
Contributor
Author
There was a problem hiding this comment.
💬 GitHub won't let the PR author self-approve via the API, so posting as Comment — verdict is otherwise 🎉 Approve (no 🐛 findings, all 4 confirmed findings are
Deep pass over every changed file, plus process conformance (ADR-0010/0011, PLAN-0010, docs, testing.md). This one's genuinely well put together — thorough ADRs with real considered-alternatives, docs updated in the same PR, extensive test coverage (build clean, 210 tests across Engine/Persistence/Ruleset.Rpg pass locally at head 5e89d58), and the Rpg.Tests break from the new hard DI dependency was caught and fixed in-PR.
4 inline findings, none blocking:
⚠️ HelpTopicEditCommand: case-insensitive lookup followed by an unconditionaltopic.Key = keyreassignment — silently churns stored casing today, and will silently rename a topic's canonical key via an alias-typed edit once aliases become settable.⚠️ HelpCommand's keyword tier passes the whole joined query as one "keyword" toFindByKeywordAsync, which only exact-matches a single entry — multi-word queries can never hit tier 2.⚠️ HelpTopicChunker's paragraph-blank-line chunking is unreachable via the only authoring path (single-line Telnet command collapses all whitespace) — every topic chunks to 1 piece today. Worth adding to PLAN-0010/docs' Open Items.- 📝 No unique index on
HelpTopic.Key— a check-then-act race under concurrent connections could produce duplicate keys. Low-probability, suggestion only.
None of these lose data, break an existing invariant, or fail a test — they're logic gaps in genuinely new functionality that the PR is already upfront about having open items in.
- 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.
Contributor
Author
|
Re: #21 (review) — all 3 inline findings from this review addressed in 944ada4 (replied individually on each thread) and resolved. |
Contributor
Author
|
Re: #21 (review) — all 4 findings from this review addressed in 944ada4: 3 fixed in code (Key reassignment, keyword-tier tokenization, unique Key index) with regression tests, 1 documented rather than code-fixed (HelpTopicChunker's paragraph chunking being unreachable via the v1 helptopic command — added to PLAN-0010's Open questions and docs/help-system.md's Open Items, since a real fix needs its own design pass). Replied individually on each of the 4 inline threads and resolved them. |
ncipollina
commented
Jul 24, 2026
ncipollina
commented
Jul 24, 2026
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚀 Pull Request
📋 Summary
Slice 5 of the WheelMUD reconciliation roadmap:
help <query>now resolves a persistedHelpTopicinstead of only listing commands, through a three-tier pipeline (exact name/alias → keyword → threshold-gated semantic-search fallback). Content is authored entirely in-game (helptopic/helpindex,MinorBuilder-gated) — no help-file format, matching the existing OLC precedent rather than WheelMUD's file-basedHelpManager. Also swaps the project icon and adds an ASCII banner to the README/docsite, since we were in there anyway.📝 Changes
Help system core (ADR-0010, PLAN-0010)
SharpMud.Engine/Help/domain:HelpTopic/HelpTopicChunk/HelpTopicId,IHelpRepository,IEmbeddingProvider+StubEmbeddingProvider(deterministic hashed-bag-of-words, no external dependency),IHelpSearchIndex+CosineHelpSearchIndex(brute-force cosine, storage-agnostic),HelpTopicChunker.HelpCommandextended with the three-tier lookup pipeline; no-argument command listing unchanged.helptopic <key> <body>/helpindex rebuild(SecurityRole.MinorBuilder), registered viaHelpAdminCommands.HelpTopic/HelpTopicChunkEF configs (HelpRepository, delete-then-reinsert shape mirroringThingRepository), wired intoAddSharpMudSqlitePersistencealongsideIThingRepository.AddSharpMudRuleset/BuiltinCommands.RegisterAllnow requireIHelpRepository/IHelpSearchIndex— a new hard dependency for any consumer, sincehelpis always registered.Real local embedding provider for the sample (ADR-0011)
LocalEmbeddingProviderinSharpMud.Samples.ClassicwrapsSmartComponents.LocalEmbeddings(ONNX Runtime, no cloud API/key, model downloaded at build time). Registered afterAddSharpMudSqlitePersistenceto override the stub.SharpMud.Engine/Persistencegain no ML dependency — it's sample-only.IEmbeddingProvider.RelevanceThresholdmoved from a hardcoded constant inCosineHelpSearchIndexonto the provider — different models produce cosine scores on very different scales, discovered by actually running the real model and finding false positives (see Validation).Docs
docs/help-system.md(internal/engineering) anddocsite/docs/help-system.md(public-facing, in themoderation-and-world-building.mdstyle), added to the docsite nav.docs/commands.md,docsite/docs/customizing.mdupdated; roadmap/ADR/plan indices updated;docs/research/wheelmud-findings.mdrecords what's adopted vs. deliberately deviated from WheelMUD'sHelpManager.Branding
icon.png(NuGetPackageIcon) anddocsite/docs/images/icon.pngreplaced with a new logo. ASCII banner added toREADME.md(next to the icon) anddocsite/docs/index.md.🧪 Validation
LocalEmbeddingProviderin place of the stub:help wizard/help sorcerer/a full natural-language paraphrase all correctly resolved a topic that never contains those words;help rusty swordcorrectly returned no match.0.5) for the real model lethelp up(a movement verb, no help topic exists) incorrectly match an unrelated topic — short, generic single-word queries scored higher (~0.54) than longer unrelated phrases (~0.33–0.40) did. Raised to0.58and re-verifiedhelp up/help who/help northall correctly report no match while real matches still succeed. Full numbers and reasoning in ADR-0011.BuiltinCommands.RegisterAllandAddSharpMudRulesetnow requireIHelpRepository/IHelpSearchIndexto be registered in DI (both are registered byAddSharpMudSqlitePersistence, so any existing consumer already calling that gets them for free — but a consumer building a minimalServiceCollectionby hand, e.g. in a test, needs to add them). Caught and fixed one existing test (SharpMud.Ruleset.Rpg.Tests) that needed this.💬 Notes for Reviewers
HelpTopicand persistence fully support them, buthelptopiconly takes<key> <body>. Called out as an open item in PLAN-0010, deliberately kept out of this slice's scope.LocalEmbeddingProvider'sRelevanceThreshold = 0.58is empirically tuned against one small test corpus, not derived from a formula — flagged in code/docs as something to revisit with real content and real player queries.SmartComponents.LocalEmbeddingsis a preview-only package from an experimental Microsoft repo (see ADR-0011's Negative Consequences) — scoped to the sample project only, deliberate.