From 99b2e3901006d7041b561dcc8a361cb7b92c7426 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Fri, 31 Jul 2026 10:43:41 -0500 Subject: [PATCH] A facet has three states: filter in, filter out, or do not care FacetChoice was a value or nothing, so the panel could ask "only Evennia" and could not ask "anything but Evennia" -- a question the catalogue can answer and the interface had no way to put. It now carries a polarity, and every choice facet gets it at once: charset, codebase, family, genre and language, plus the codebase-family filter the reference pages link in on. Null still means the facet is not being asked about, which is the state that must not be confused with either of the others. The polarity is applied ONCE, TO THE ANSWER, and never per token. A facet can hand one row several tokens -- a game reached an hour ago is in the last day, the last week and the last month -- so inverting the comparison per token would make an exclusion mean "some token differs", which every multi-token row satisfies. Covers() compares the value and Admits() applies the polarity, and they are separate methods so the call site cannot collapse them by accident. An excluded value does not drop the games that have no value for the facet. The tempting reading of "not Evennia" is "has a codebase, and it is not Evennia", which quietly discards every game whose codebase we never identified -- turning a gap in our own measurement into a property of those games. Not identifying a codebase is a measurement, and it is not a measurement of being Evennia. The unknown can itself be excluded, which is the separate question of "games whose codebase we did identify", and there is a test for each. In a URL an exclusion is a leading bang, and a value that genuinely starts with one is doubled so it stays reachable rather than reading as its own negation. The panel offers both directions as two s in the same select -- the only way to carry a third state in a plain GET form with no script -- so the question is still entirely in the URL and a filtered listing is still linkable. One bug worth recording. Invert() is a method rather than a property because a record's generated ToString prints every public property, so a property returning another FacetChoice recursed until the stack ran out -- and it surfaced as an unrelated test dying inside an assertion message, with nothing pointing at facets. Verified against the real catalogue: every polarity pair partitions it exactly. 2 PennMUSH and 14 not, 1 Evennia and 15 not, 5 with no identified codebase and 11 with one, against 16 games. Co-Authored-By: Claude Opus 5 --- src/MUI.Catalog/Facets.cs | 147 ++++++++++++++-- src/MUI.Catalog/Views.cs | 9 +- src/MUI.Web/Api/ApiModels.cs | 2 +- src/MUI.Web/Api/GameFilterBinding.cs | 4 +- src/MUI.Web/Components/FacetPanel.razor | 44 ++++- src/MUI.Web/Components/Pages/Games.razor | 4 +- src/MUI.Web/Reference/ReferenceFigures.cs | 2 +- tests/MUI.Catalog.Tests/FacetPolarityTests.cs | 163 ++++++++++++++++++ tests/MUI.Web.Tests/ReferenceFiguresTests.cs | 5 +- 9 files changed, 352 insertions(+), 28 deletions(-) create mode 100644 tests/MUI.Catalog.Tests/FacetPolarityTests.cs diff --git a/src/MUI.Catalog/Facets.cs b/src/MUI.Catalog/Facets.cs index 1c99726..f2cf7fc 100644 --- a/src/MUI.Catalog/Facets.cs +++ b/src/MUI.Catalog/Facets.cs @@ -88,7 +88,7 @@ public enum FacetKind /// real and useful question; it is not "games with no codebase", and neither is it a games-with-any /// filter left blank. Modelling it as a value would let one be typed where the other was meant. /// -public sealed record FacetChoice(string? Value) +public sealed record FacetChoice(string? Value, bool Exclude = false) { /// /// The querystring spelling of the absence. Tilde-prefixed so it cannot collide with a real @@ -96,22 +96,89 @@ public sealed record FacetChoice(string? Value) /// public const string UnknownToken = "~unknown"; + /// + /// The prefix that turns a selection inside out: ?codebase=!Evennia is every game whose + /// codebase is not Evennia. + /// + /// + /// + /// A facet has three states, not two, and the third one is what makes the panel a filter + /// rather than a set of shortcuts. Absent means the facet is not being asked about; a value + /// means only these; an excluded value means anything but these. Without the + /// third, "show me the games that are not Evennia" is a question the catalogue can answer and + /// the interface cannot ask. + /// + /// + /// ! rather than - because a codebase, genre or language may legitimately begin + /// with a hyphen and none of the values observed in the wild begin with a bang. A literal + /// leading ! is written !!, so a value is never unreachable — see + /// . + /// + /// + public const string ExcludeToken = "!"; + /// Games for which this facet has no value at all. public static readonly FacetChoice Unknown = new((string?)null); public static FacetChoice Of(string value) => new(value); + /// The same selection, inside out. + public static FacetChoice Not(string value) => new(value, Exclude: true); + public bool IsUnknown => Value is null; - /// What this selection is called in a URL. - public string Token => Value ?? UnknownToken; + /// What this selection is called in a URL, polarity included. + public string Token => + (Exclude ? ExcludeToken : string.Empty) + Escaped(Value ?? UnknownToken); - public static FacetChoice Parse(string token) => - string.Equals(token, UnknownToken, StringComparison.Ordinal) ? Unknown : Of(token); + /// The same facet with its polarity flipped, which is what a panel's toggle emits. + /// + /// A method and not a property, deliberately. A record's generated ToString prints + /// every public property, so a property returning another makes + /// printing one recurse until the stack runs out — which is exactly what happened, and it + /// surfaced as an unrelated test dying inside an assertion message rather than as anything to do + /// with facets. + /// + public FacetChoice Invert() => this with { Exclude = !Exclude }; - /// Whether a game whose value for this facet is matches. - public bool Matches(string? actual) => + public static FacetChoice Parse(string token) + { + ArgumentNullException.ThrowIfNull(token); + + var exclude = token.StartsWith(ExcludeToken, StringComparison.Ordinal) + && !token.StartsWith(ExcludeToken + ExcludeToken, StringComparison.Ordinal); + + var body = exclude + ? token[ExcludeToken.Length..] + // A doubled bang is a literal one: a value that genuinely starts with "!" stays + // reachable rather than being silently reinterpreted as its own negation. + : token.StartsWith(ExcludeToken + ExcludeToken, StringComparison.Ordinal) + ? token[ExcludeToken.Length..] + : token; + + return string.Equals(body, UnknownToken, StringComparison.Ordinal) + ? Unknown with { Exclude = exclude } + : new FacetChoice(body, exclude); + } + + /// + /// Whether is the value this selection names — polarity ignored. + /// + /// + /// Deliberately separate from . A facet can hand a row several tokens (a + /// game reached an hour ago is in the last day, week and month), and inverting the comparison + /// per token would make an excluded selection mean "some token differs" — which every row with + /// more than one token satisfies. The polarity is applied once, to the answer. + /// + public bool Covers(string? actual) => IsUnknown ? actual is null : string.Equals(actual, Value, StringComparison.OrdinalIgnoreCase); + + /// Whether a row that did or did not match this selection survives it. + public bool Admits(bool covered) => covered != Exclude; + + /// A value that begins with the exclusion marker is doubled so it round-trips. + private static string Escaped(string value) => + value.StartsWith(ExcludeToken, StringComparison.Ordinal) ? ExcludeToken + value : value; } /// @@ -151,7 +218,36 @@ public enum LastSeenBand /// results it will not deliver. A value nothing matches is never offered at all — the one exception /// is a value that is currently selected, which stays visible at zero so it can be seen and undone. /// -public sealed record FacetValue(string Token, int Count, bool IsSelected, bool IsUnknown); +public sealed record FacetValue( + string Token, + int Count, + bool IsSelected, + bool IsUnknown, + bool IsExcluded = false) +{ + /// + /// The three states a value can be in, as one question a renderer can switch on. + /// + /// + /// A panel that only knew would draw an included and an excluded value + /// identically, which is the one thing a tri-state filter must not do — a reader would have no + /// way to tell "only Evennia" from "anything but Evennia" except by reading the URL. + /// + public FacetState State => (IsSelected, IsExcluded) switch + { + (true, true) => FacetState.Excluded, + (true, false) => FacetState.Included, + _ => FacetState.Unselected, + }; +} + +/// Whether a facet value is being filtered in, filtered out, or not asked about. +public enum FacetState +{ + Unselected, + Included, + Excluded, +} /// One facet, ready to render: what it is called, what it reads, and what it offers. /// @@ -257,7 +353,7 @@ public static GameListing Search(IReadOnlyList rows, GameFilter fi var baseRows = rows .Where(r => (wantsArchived || r.Band is not ActivityBand.Archived) && MatchesText(r, filter.Text) - && CodebaseFamily.Matches(r.Codebase, filter.CodebaseFamily)) + && AdmitsFamily(r, filter.CodebaseFamily)) .ToList(); var results = baseRows.Where(r => Chosen(r, filter, null) && Present(r, filter)).ToList(); @@ -324,6 +420,18 @@ private static bool MatchesText(GameFacetRow row, string? text) || (row.Codebase?.Contains(needle, StringComparison.OrdinalIgnoreCase) ?? false); } + /// + /// Whether a row survives the codebase-family filter, polarity included. + /// + /// + /// Separate from the choice facets because the test is a bounded prefix rather than an equality, + /// and because this is a filter rather than a counted facet — it narrows the set the panel's + /// counts are taken over, which is what makes a codebase page's facet counts counts within that + /// codebase. + /// + private static bool AdmitsFamily(GameFacetRow row, FacetChoice? family) => + family is null || family.Admits(CodebaseFamily.Matches(row.Codebase, family.Value)); + private static bool Chosen(GameFacetRow row, GameFilter filter, string? except) { foreach (var facet in Choices) @@ -333,8 +441,9 @@ private static bool Chosen(GameFacetRow row, GameFilter filter, string? except) continue; } + // Applied once to the answer, not per token: see FacetChoice.Covers. if (facet.SelectionOf(filter) is { } selection - && !facet.TokensOf(row).Any(selection.Matches)) + && !selection.Admits(facet.TokensOf(row).Any(selection.Covers))) { return false; } @@ -416,8 +525,9 @@ .. vocabulary .Select(token => new FacetValue( token, counts.GetValueOrDefault(token), - selection?.Matches(token) ?? false, - IsUnknown: false)) + selection?.Covers(token) ?? false, + IsUnknown: false, + IsExcluded: (selection?.Covers(token) ?? false) && selection!.Exclude)) .Where(v => v.Count > 0 || v.IsSelected), ]; } @@ -442,7 +552,11 @@ private static List Open( var named = counts .Where(c => !string.Equals(c.Key, FacetChoice.UnknownToken, StringComparison.Ordinal)) .Select(c => new FacetValue( - c.Key, c.Value, selection?.Matches(c.Key) ?? false, IsUnknown: false)) + c.Key, + c.Value, + selection?.Covers(c.Key) ?? false, + IsUnknown: false, + IsExcluded: (selection?.Covers(c.Key) ?? false) && selection!.Exclude)) .OrderByDescending(v => v.IsSelected) .ThenByDescending(v => v.Count) .ThenBy(v => v.Token, StringComparer.Ordinal) @@ -456,7 +570,12 @@ private static List Open( if (unknown > 0 || unknownSelected) { - named.Add(new FacetValue(FacetChoice.UnknownToken, unknown, unknownSelected, IsUnknown: true)); + named.Add(new FacetValue( + FacetChoice.UnknownToken, + unknown, + unknownSelected, + IsUnknown: true, + IsExcluded: unknownSelected && selection!.Exclude)); } return named; diff --git a/src/MUI.Catalog/Views.cs b/src/MUI.Catalog/Views.cs index a0f8b03..47c95b8 100644 --- a/src/MUI.Catalog/Views.cs +++ b/src/MUI.Catalog/Views.cs @@ -188,8 +188,15 @@ public sealed record GameFilter /// variable, which answers TinyMUD or DikuMUD; this is the codebase with its /// version taken off. A reference page for PennMUSH wants the third and neither of the others. /// + /// + /// A for its polarity rather than its matching: the choice carries the + /// value and whether it is being filtered in or out, and the test is supplied by the + /// caller — , a bounded prefix, so ROM does not gather + /// ROMulus. It is not offered as a counted facet in the panel, so it never appears in the + /// vocabulary the choice facets are drawn from. + /// /// - public string? CodebaseFamily { get; init; } + public FacetChoice? CodebaseFamily { get; init; } } /// diff --git a/src/MUI.Web/Api/ApiModels.cs b/src/MUI.Web/Api/ApiModels.cs index 1def9cf..1abfe5e 100644 --- a/src/MUI.Web/Api/ApiModels.cs +++ b/src/MUI.Web/Api/ApiModels.cs @@ -213,7 +213,7 @@ public static FilterView Of(GameFilter filter) filter.Family?.Token, filter.Genre?.Token, filter.Language?.Token, - filter.CodebaseFamily); + filter.CodebaseFamily?.Token); } } diff --git a/src/MUI.Web/Api/GameFilterBinding.cs b/src/MUI.Web/Api/GameFilterBinding.cs index 44d9545..4803263 100644 --- a/src/MUI.Web/Api/GameFilterBinding.cs +++ b/src/MUI.Web/Api/GameFilterBinding.cs @@ -89,7 +89,9 @@ private static bool TryRead( Family = Choice(read, FacetKeys.Family), Genre = Choice(read, FacetKeys.Genre), Language = Choice(read, FacetKeys.Language), - CodebaseFamily = string.IsNullOrWhiteSpace(codebaseFamily) ? null : codebaseFamily.Trim(), + CodebaseFamily = string.IsNullOrWhiteSpace(codebaseFamily) + ? null + : FacetChoice.Parse(codebaseFamily.Trim()), }; result = new GameQuery( diff --git a/src/MUI.Web/Components/FacetPanel.razor b/src/MUI.Web/Components/FacetPanel.razor index 2115076..b9bff4c 100644 --- a/src/MUI.Web/Components/FacetPanel.razor +++ b/src/MUI.Web/Components/FacetPanel.razor @@ -38,15 +38,36 @@ @FacetWords.Group(group.Key) @FacetWords.Evidence(group.Evidence) + @* + Every value appears twice — once to filter in, once to filter out — which + is the only way to offer the third state in a plain GET form with no + script. A facet has three states (spec §9 and FacetChoice): not asked + about, only these, anything but these. Two s rather than a + second control, so one @* Always first, always empty: a facet you cannot un-choose is a trap. *@ - @foreach (var value in group.Values) - { - - } + + @foreach (var value in group.Values) + { + + } + + + @foreach (var value in group.Values) + { + + } + } @@ -88,6 +109,17 @@ @code { + /// + /// The same value, spelled as an exclusion — Evennia becomes !Evennia. + /// + /// + /// Built through rather than by prefixing a string here, so the + /// panel's spelling and the binding's parser cannot drift: a value that itself begins with the + /// marker is escaped by the same code that unescapes it. + /// + private static FacetChoice Excluded(FacetValue value) => + (value.IsUnknown ? FacetChoice.Unknown : FacetChoice.Of(value.Token)) with { Exclude = true }; + [Parameter, EditorRequired] public IReadOnlyList Facets { get; set; } = []; [Parameter, EditorRequired] public GameFilter Filter { get; set; } = new(); diff --git a/src/MUI.Web/Components/Pages/Games.razor b/src/MUI.Web/Components/Pages/Games.razor index 9689e31..aa7096b 100644 --- a/src/MUI.Web/Components/Pages/Games.razor +++ b/src/MUI.Web/Components/Pages/Games.razor @@ -30,14 +30,14 @@ else { - @if (Filter.CodebaseFamily is { } family) + @if (Filter.CodebaseFamily is { Exclude: false, Value: { } family }) { @* The filter a reference page links in on. It is a filter and not a search, so it says which family it is showing and offers the way back out — a reader who arrived from /reference/codebases/pennmush should not have to guess why the listing is short. *@

codebase @family · - what this codebase is · + what this codebase is · every game

} diff --git a/src/MUI.Web/Reference/ReferenceFigures.cs b/src/MUI.Web/Reference/ReferenceFigures.cs index 6de4067..ccfdf04 100644 --- a/src/MUI.Web/Reference/ReferenceFigures.cs +++ b/src/MUI.Web/Reference/ReferenceFigures.cs @@ -33,7 +33,7 @@ public static async Task ReadAsync( ArgumentNullException.ThrowIfNull(queries); var games = await queries.ListAsync( - new GameFilter { CodebaseFamily = family, IncludeArchived = true }, + new GameFilter { CodebaseFamily = FacetChoice.Of(family), IncludeArchived = true }, cancellationToken); return new CodebaseFigures( diff --git a/tests/MUI.Catalog.Tests/FacetPolarityTests.cs b/tests/MUI.Catalog.Tests/FacetPolarityTests.cs new file mode 100644 index 0000000..4426bcc --- /dev/null +++ b/tests/MUI.Catalog.Tests/FacetPolarityTests.cs @@ -0,0 +1,163 @@ +using MUI.Catalog; + +namespace MUI.Catalog.Tests; + +/// +/// The third state a facet can be in: filtered out (spec §9). +/// +/// +/// Absent means the facet is not being asked about, a value means only these, and an +/// excluded value means anything but these. Without the third, "show me the games that are +/// not Evennia" is a question the catalogue can answer and the interface cannot ask. +/// +public class FacetPolarityTests +{ + private static readonly GameSummary Penn = Game("penn", "PennMUSH 1.8.8p0"); + private static readonly GameSummary Evennia = Game("evennia", "Evennia"); + private static readonly GameSummary Rom = Game("rom", "ROM"); + private static readonly GameSummary Nameless = Game("nameless", null); + + [Test] + public async Task ExcludingAValueReturnsEverythingElse() + { + var listing = Search(new GameFilter { Codebase = FacetChoice.Not("Evennia") }); + + await Assert.That(listing.Games.Select(g => g.Slug)) + .IsEquivalentTo(new[] { "penn", "rom", "nameless" }); + } + + [Test] + public async Task IncludingAValueStillReturnsOnlyIt() + { + var listing = Search(new GameFilter { Codebase = FacetChoice.Of("Evennia") }); + + await Assert.That(listing.Games.Select(g => g.Slug)).IsEquivalentTo(new[] { "evennia" }); + } + + [Test] + public async Task NoSelectionFiltersOnNothing() + { + await Assert.That(Search(new GameFilter()).Games.Count).IsEqualTo(4); + } + + /// + /// A game with no value for the facet survives an exclusion, because it is not the thing excluded. + /// + /// + /// The tempting bug is to treat "not Evennia" as "has a codebase, and it is not Evennia", which + /// would quietly drop every game whose codebase we never identified — turning our own gap in + /// measurement into a property of those games. Not identifying a codebase is a measurement, and + /// it is not a measurement of being Evennia. + /// + [Test] + public async Task AGameWithNoValueSurvivesAnExclusion() + { + var listing = Search(new GameFilter { Codebase = FacetChoice.Not("Evennia") }); + + await Assert.That(listing.Games.Select(g => g.Slug)).Contains("nameless"); + } + + /// Excluding the unknown is its own question, and answers it. + [Test] + public async Task TheUnknownCanItselfBeExcluded() + { + var listing = Search(new GameFilter + { + Codebase = FacetChoice.Unknown with { Exclude = true }, + }); + + await Assert.That(listing.Games.Select(g => g.Slug)) + .IsEquivalentTo(new[] { "penn", "evennia", "rom" }); + } + + /// The family filter is a bounded prefix, and it inverts the same way. + [Test] + public async Task ExcludingACodebaseFamilyTakesEveryPatchlevelWithIt() + { + var listing = Search(new GameFilter { CodebaseFamily = FacetChoice.Not("PennMUSH") }); + + // PennMUSH 1.8.8p0 goes with the family it belongs to; ROM stays, and is not caught by a + // prefix that would have gathered ROMulus. + await Assert.That(listing.Games.Select(g => g.Slug)) + .IsEquivalentTo(new[] { "evennia", "rom", "nameless" }); + } + + /// A token round-trips through the URL with its polarity intact. + [Test] + public async Task PolaritySurvivesTheQuerystring() + { + foreach (var choice in new[] + { + FacetChoice.Of("Evennia"), + FacetChoice.Not("Evennia"), + FacetChoice.Unknown, + FacetChoice.Unknown with { Exclude = true }, + }) + { + await Assert.That(FacetChoice.Parse(choice.Token)).IsEqualTo(choice); + } + } + + /// + /// A value that begins with the marker stays reachable rather than reading as its own negation. + /// + [Test] + public async Task AValueBeginningWithTheMarkerIsNotMistakenForAnExclusion() + { + var literal = FacetChoice.Of("!important"); + + await Assert.That(literal.Token).IsEqualTo("!!important"); + await Assert.That(FacetChoice.Parse(literal.Token)).IsEqualTo(literal); + await Assert.That(FacetChoice.Parse(literal.Token).Exclude).IsFalse(); + } + + /// + /// Polarity is applied to the answer, never per token. + /// + /// + /// A facet can hand one row several tokens — a game reached an hour ago is in the last day, the + /// last week and the last month. Inverting the comparison per token would make an excluded + /// selection mean "some token differs", which every multi-token row satisfies, so "not seen in + /// the last day" would return everything. + /// + [Test] + public async Task AMultiTokenFacetInvertsOnceRatherThanPerToken() + { + var choice = FacetChoice.Not("day"); + + await Assert.That(choice.Admits(covered: true)).IsFalse(); + await Assert.That(choice.Admits(covered: false)).IsTrue(); + await Assert.That(FacetChoice.Of("day").Admits(covered: true)).IsTrue(); + } + + /// The panel can tell the three states apart, which is what lets it draw them apart. + [Test] + public async Task AFacetValueReportsWhichOfTheThreeStatesItIsIn() + { + var listing = Search(new GameFilter { Codebase = FacetChoice.Not("Evennia") }); + var codebase = listing.Facets.Single(f => f.Key == FacetKeys.Codebase); + + await Assert.That(codebase.Values.Single(v => v.Token == "Evennia").State) + .IsEqualTo(FacetState.Excluded); + await Assert.That(codebase.Values.Single(v => v.Token == "ROM").State) + .IsEqualTo(FacetState.Unselected); + } + + private static GameListing Search(GameFilter filter) => + FacetedSearch.Search([.. new[] { Penn, Evennia, Rom, Nameless }.Select(Row)], filter); + + private static GameFacetRow Row(GameSummary game) => new( + game, + ActivityBand.Quiet, + LastSeenBand.Week, + TlsMeasured: false, + Charset: null, + Language: null, + Codebase: game.Codebase, + Family: null, + Genre: null); + + private static GameSummary Game(string slug, string? codebase) => new( + Guid.NewGuid(), slug, slug, null, LifecycleState.Active, IsClaimed: false, + PlayersNow: 1, Codebase: codebase, MeasuredProtocols: []); +} diff --git a/tests/MUI.Web.Tests/ReferenceFiguresTests.cs b/tests/MUI.Web.Tests/ReferenceFiguresTests.cs index 8a12a3e..ad31192 100644 --- a/tests/MUI.Web.Tests/ReferenceFiguresTests.cs +++ b/tests/MUI.Web.Tests/ReferenceFiguresTests.cs @@ -121,7 +121,7 @@ public async Task TheCodebaseLinkAndTheCountAreOneFilter() var queries = new FixtureGameQueries(); var figures = await CodebaseFigures.ReadAsync(queries, page.Codebase!); - var listing = await queries.ListAsync(new GameFilter { CodebaseFamily = page.Codebase }); + var listing = await queries.ListAsync(new GameFilter { CodebaseFamily = FacetChoice.Of(page.Codebase!) }); await Assert.That(figures.Listed).IsEqualTo(listing.Count); await Assert.That(page.GamesPath).IsEqualTo("/games?codebase-family=Evennia"); @@ -144,7 +144,8 @@ public Task> ListAsync( [ .. games .Where(g => filter.IncludeArchived || g.State is not LifecycleState.Archived) - .Where(g => CodebaseFamily.Matches(g.Codebase, filter.CodebaseFamily)), + .Where(g => filter.CodebaseFamily is not { } family + || family.Admits(CodebaseFamily.Matches(g.Codebase, family.Value))), ]); public Task FindAsync(string slug, CancellationToken cancellationToken = default) =>