diff --git a/src/MeshWeaver.Documentation/Data/Architecture/SearchCoverageAndRefusal.md b/src/MeshWeaver.Documentation/Data/Architecture/SearchCoverageAndRefusal.md index 45da686ca8..5886425272 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/SearchCoverageAndRefusal.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/SearchCoverageAndRefusal.md @@ -178,6 +178,34 @@ an empty list from one provider must not vouch for partitions another provider n `basePath` now becomes `namespace:{base} scope:descendants` unless the query states its own `scope:`. "Search from here" means the subtree. +### `basePath` never rewrites the query's own location + +A query that names its own location (`path:`, `namespace:`, a membership or wildcard namespace) +**keeps** it when that location lies at or under the base — it is the narrower statement — and is +**refused**, naming both, when it lies outside: no answer can honour both. Only a query that names +no location of its own (an exclusion like `-namespace:X` is a filter, not a location; an empty +`namespace:` names nothing) is scoped by the base. + +Three edges keep that honest: + +- **A location must bound the WHOLE query.** `namespace:/* OR nodeType:Markdown` names a + location under the base in its left branch only; the right branch would read everywhere, so the + query is refused. The test is the sufficiency predicate itself (an anchor, or a namespace filter + on every `OR` branch), with `partitions:all` set aside. +- **A multi-value exclusion is a filter too.** The parser emits `-namespace:A|B` (and + `-namespace:(A OR B)`) as a `namespace NotIn (…)` filter and leaves the anchor and scope alone; it + used to defer it with the positive alternation, which cleared the anchor — so the composed query + lost `namespace:` and was refused as unanchored, or answered from outside the base. +- **The rest of the text is kept verbatim.** Dropping an empty `namespace:` is quote-aware: a + split-and-rejoin collapsed the spaces in `name:"Jane Q Example"` and dropped a `namespace:` + written inside a quoted value. + +Before #6828 the tool deleted every `namespace:` from the text before prefixing the base, so +`namespace: name:"…"` under `basePath: @` became the FREE-TEXT term `` +read inside A — a semantic search flooded with whatever mentioned the words, reporting +`coverage.partitions: [""]` for a query that named B — and `-namespace:X` became a search +FOR X. The coverage was honest about where it read; the composition was wrong about where to read. + ## Reading a zero — how to state the denominator The pre-deploy NodeType sweep is now written in the declared form, and its zero is stated against diff --git a/src/MeshWeaver.Mesh.Contract/Query/QueryParser.cs b/src/MeshWeaver.Mesh.Contract/Query/QueryParser.cs index 367ebdd501..06da7eddba 100644 --- a/src/MeshWeaver.Mesh.Contract/Query/QueryParser.cs +++ b/src/MeshWeaver.Mesh.Contract/Query/QueryParser.cs @@ -507,7 +507,6 @@ private string ParseSingleValue(string input, ref int i, bool stopAtParens, out // namespaces when scope:selfAndAncestors. This is what lets a SINGLE query union // built-in + per-context + per-NodeType + per-user instances (see AgentPickerProjection). string[]? namespaceAlternation = null; - bool namespaceAlternationNegated = false; // Where each wildcard `namespace:*/X` LIKE filter landed in filterTokens, so the post-loop // pass can widen it to the SUBTREE once `scope:` is known (the qualifier may appear AFTER // the namespace token). See WidenWildcardNamespacesToSubtree. @@ -609,11 +608,25 @@ or QueryOperator.NotEqual // Alternation `namespace:A|B|C` (parsed to In/NotIn with >1 values) is an EXACT // MEMBERSHIP constraint across the listed namespaces, not a single base namespace // + scope walk. Deferred to post-loop resolution (a `namespace IN (...)` filter). - if (token.Condition!.Operator is QueryOperator.In or QueryOperator.NotIn - && token.Condition.Values.Length > 1) + // + // 🚨 A NEGATED alternation `-namespace:A|B` is an EXCLUSION FILTER, never the + // namespace constraint (#6828, review of #6837): deferring it with the positive + // form cleared the query's anchor and reset its scope in the post-loop pass, so + // `namespace:X -namespace:X/A|X/B` lost `X` — refused as unanchored, or, with + // `partitions:all`, answered from outside `X`. It is emitted here as the + // `namespace NotIn (...)` filter the deferred pass would have produced, and + // leaves the anchor and scope alone, like the single `-namespace:X` below. + if (token.Condition!.Operator is QueryOperator.NotIn && token.Condition.Values.Length > 1) + { + filterTokens.Add(new Token(TokenType.Comparison, string.Empty, + new QueryCondition("namespace", QueryOperator.NotIn, + token.Condition.Values.Select(v => v.Trim('/')) + .Distinct(StringComparer.OrdinalIgnoreCase).ToArray()))); + continue; + } + if (token.Condition.Operator is QueryOperator.In && token.Condition.Values.Length > 1) { namespaceAlternation = token.Condition.Values; - namespaceAlternationNegated = token.Condition.Operator == QueryOperator.NotIn; namespaceUsed = true; continue; } @@ -813,7 +826,7 @@ or QueryOperator.NotEqual .Distinct(StringComparer.OrdinalIgnoreCase).ToArray(); filterTokens.Add(new Token(TokenType.Comparison, string.Empty, new QueryCondition("namespace", - namespaceAlternationNegated ? QueryOperator.NotIn : QueryOperator.In, distinct))); + QueryOperator.In, distinct))); path = null; scope = QueryScope.Exact; } diff --git a/src/MeshWeaver.Mesh.Operations/MeshOperations.cs b/src/MeshWeaver.Mesh.Operations/MeshOperations.cs index a2bb23c90c..d1403c1457 100644 --- a/src/MeshWeaver.Mesh.Operations/MeshOperations.cs +++ b/src/MeshWeaver.Mesh.Operations/MeshOperations.cs @@ -1450,14 +1450,67 @@ public IObservable Search(string query, string? basePath = null, int lim } else { - var cleanQuery = query.Replace("namespace:", "").Trim(); - // "Base path to search FROM" is the whole subtree, not one level of it. A bare - // `namespace:X` is IMMEDIATE CHILDREN, so `basePath: @Admin` + `nodeType:LogIncident` - // answered 0 for nodes at `Admin/_LogIncident/…` — the parameter whose documented - // purpose is scoping did not reach what it scoped to (#4274, property 1). A query that - // states its own scope keeps it. - var scoped = StatesScope(cleanQuery) ? cleanQuery : $"{cleanQuery} scope:descendants".Trim(); - fullQuery = $"namespace:{resolvedBase} {scoped}".Trim(); + // 🚨 The query's OWN location is never rewritten (#6828). This used to strip every + // `namespace:` out of the text before prefixing the base, so `namespace:acme/Deals …` + // under `basePath: @beta` became the FREE-TEXT term `acme/Deals`, read against beta's + // partition: a semantic search flooded with whatever mentioned the words, with + // `coverage.partitions: ["beta"]` for a query that named acme — and `-namespace:X`, an + // exclusion, became a search FOR X. Now a query that states its own location keeps it + // when that location lies under the base (it is the narrower statement) and is + // refused, naming both, when it does not; only a query that names no location of its + // own is scoped by the base. + var own = new QueryParser().Parse(query); + var ownLocations = OwnLocations(own); + if (ownLocations is null) + { + // "Base path to search FROM" is the whole subtree, not one level of it. A bare + // `namespace:X` is IMMEDIATE CHILDREN, so `basePath: @Admin` + `nodeType:LogIncident` + // answered 0 for nodes at `Admin/_LogIncident/…` — the parameter whose documented + // purpose is scoping did not reach what it scoped to (#4274, property 1). A query that + // states its own scope keeps it. + // An EMPTY `namespace:`/`path:` names no location (callers write `namespace:` with a + // basePath to mean "the children of the base") — drop it, or it would re-anchor the + // composed query at the root. + // Quote-aware, and the rest of the text verbatim: a split-and-rejoin collapsed the + // spaces inside `name:"Jane Q Example"` and dropped a `namespace:` written INSIDE a + // quoted value, so an exact-name search ran on a different value. + var trimmed = WithoutEmptyLocationQualifiers(query); + var scoped = StatesScope(trimmed) ? trimmed : $"{trimmed} scope:descendants".Trim(); + fullQuery = $"namespace:{resolvedBase} {scoped}".Trim(); + } + else + { + var outside = ownLocations.Where(l => !IsAtOrUnder(l, resolvedBase)).ToList(); + if (outside.Count > 0) + { + logger.LogInformation( + "Search refused: query {Query} names locations {Outside} outside basePath {BasePath}", + query, outside, basePath); + return Observable.Return( + $"Error: basePath '{resolvedBase}' and the query's own location " + + $"({string.Join(", ", outside.Select(l => l.Length == 0 ? "the root" : $"'{l}'"))}) disagree — " + + "the query names a place outside the base, so no answer could honour both. " + + "Drop the basePath to search where the query says, or drop the query's " + + "path:/namespace: to search under the base."); + } + // Every named location lies under the base — but a location named in ONE `OR` + // branch bounds only that branch: `namespace:Base/* OR nodeType:Markdown` would run + // its right branch everywhere. The query keeps its own location only when the WHOLE + // expression is bounded by it (an anchor, or a namespace filter on every branch — + // the sufficiency predicate with the `partitions:all` flag set aside). + if (!(own with { CrossPartition = false }).IsSufficientlySpecified()) + { + logger.LogInformation( + "Search refused: query {Query} names a location in only some of its branches under basePath {BasePath}", + query, basePath); + return Observable.Return( + $"Error: the query names a location in only some of its OR branches, so under basePath " + + $"'{resolvedBase}' the other branches would search outside it. Give every branch a " + + "path:/namespace: under the base, drop the query's own path:/namespace: to search the " + + "whole base, or drop the basePath."); + } + fullQuery = query; + } } var parsed = new QueryParser().Parse(fullQuery); @@ -1547,6 +1600,66 @@ private static bool StatesScope(string query) => query.Split(' ', StringSplitOptions.RemoveEmptyEntries) .Any(token => token.StartsWith("scope:", StringComparison.OrdinalIgnoreCase)); + /// + /// The locations a query names on its own — its path:/namespace: anchor(s) and + /// any positive namespace filter (membership or wildcard) — or when it + /// names none and a basePath must scope it. An exclusion (-namespace:X) is a + /// filter, not a location, and is left for the base to scope (#6828). + /// + private static IReadOnlyList? OwnLocations(ParsedQuery parsed) + { + var locations = new List(); + if (parsed.Paths is { Count: > 0 } paths) + locations.AddRange(paths); + else if (!string.IsNullOrEmpty(parsed.Path)) + locations.Add(parsed.Path); + locations.AddRange(parsed.ExtractNamespacePatterns()); + return locations.Count == 0 ? null : locations.Select(l => l.Trim('/')).ToList(); + } + + /// + /// without its EMPTY namespace:/path: tokens (callers' + /// "the base's children" idiom, which names no location), the rest of the text verbatim. Quote + /// aware: whitespace and a namespace: inside a quoted value belong to the value. + /// + private static string WithoutEmptyLocationQualifiers(string query) + { + var result = new System.Text.StringBuilder(query.Length); + var i = 0; + while (i < query.Length) + { + if (char.IsWhiteSpace(query[i])) + { + result.Append(query[i++]); + continue; + } + var start = i; + var quoted = false; + while (i < query.Length && (quoted || !char.IsWhiteSpace(query[i]))) + { + if (quoted && query[i] == '\\' && i + 1 < query.Length) + i++; + else if (query[i] == '"') + quoted = !quoted; + i++; + } + var token = query[start..i]; + if (!token.Equals("namespace:", StringComparison.OrdinalIgnoreCase) + && !token.Equals("path:", StringComparison.OrdinalIgnoreCase)) + result.Append(token); + } + return result.ToString().Trim(); + } + + /// Whether is or lies below it. + private static bool IsAtOrUnder(string location, string basePath) + { + var root = basePath.Trim('/'); + return root.Length == 0 + || location.Equals(root, StringComparison.OrdinalIgnoreCase) + || location.StartsWith(root + "/", StringComparison.OrdinalIgnoreCase); + } + /// /// The coverage a can state for from the query /// alone, or when the query is unanchored and must be refused. diff --git a/test/Memex.Portal.Shared.Test/SearchBasePathKeepsTheQuerysLocationTest.cs b/test/Memex.Portal.Shared.Test/SearchBasePathKeepsTheQuerysLocationTest.cs new file mode 100644 index 0000000000..6a42008ce9 --- /dev/null +++ b/test/Memex.Portal.Shared.Test/SearchBasePathKeepsTheQuerysLocationTest.cs @@ -0,0 +1,168 @@ +using System.Reactive.Linq; +using System.Text.Json; +using MeshWeaver.AI; // MeshOperations — its namespace is a frozen binary contract (#2370) +using MeshWeaver.Fixture; +using MeshWeaver.Hosting.Monolith.TestBase; +using MeshWeaver.Mesh; +using Xunit; + +namespace Memex.Portal.Shared.Test; + +/// +/// 🚨 basePath never rewrites the query's OWN location (#6828). Measured on a portal: a +/// query anchored at one space (namespace:<space B> …) sent with basePath at +/// another reported coverage.partitions of the base's partition only — because +/// MeshOperations.Search deleted every namespace: from the text before prefixing the +/// base, turning the query's location into a FREE-TEXT term read inside the wrong partition, and +/// an exclusion (-namespace:X) into a search FOR X. +/// +/// Now a query that names its own location keeps it when it lies under the base, is refused +/// naming both when it does not, and only a query that names none is scoped by the base. +/// +public class SearchBasePathKeepsTheQuerysLocationTest(ITestOutputHelper output) : MonolithMeshTestBase(output) +{ + private const string Alpha = "BaseAlpha"; + private const string Beta = "BaseBeta"; + private const string Person = "Jane Q Example"; + + private static TimeSpan Budget => TestTimeouts.Convergence; + + protected override MeshBuilder ConfigureMesh(MeshBuilder builder) + => base.ConfigureMesh(builder).AddMeshNodes( + new MeshNode(Alpha) { Name = "Alpha", NodeType = "Markdown" }, + new MeshNode(Beta) { Name = "Beta", NodeType = "Markdown" }, + new MeshNode("Deal", Beta) { Name = "Deal", NodeType = "Markdown" }, + new MeshNode("Jane", $"{Beta}/Deal") { Name = Person, NodeType = "Markdown" }, + new MeshNode("Archive", Beta) { Name = "Archive", NodeType = "Markdown" }, + new MeshNode("Jane", $"{Beta}/Archive") { Name = Person, NodeType = "Markdown" }, + new MeshNode("Spaced", Beta) { Name = SpacedPerson, NodeType = "Markdown" }, + new MeshNode("Colon", Beta) { Name = ColonPerson, NodeType = "Markdown" }); + + private const string SpacedPerson = "Jane Q Example"; + private const string ColonPerson = "Jane namespace: Example"; + + private Task Search(string query, string? basePath, CancellationToken cancellationToken) + => new MeshOperations(Mesh).Search(query, basePath, 50) + .FirstAsync().Timeout(Budget).Await(cancellationToken); + + private static string[] Paths(string answer) + => JsonDocument.Parse(answer).RootElement.GetProperty("results").EnumerateArray() + .Select(r => r.GetProperty("path").GetString()!).OrderBy(p => p, StringComparer.Ordinal).ToArray(); + + private static string[] Partitions(string answer) + => JsonDocument.Parse(answer).RootElement.GetProperty("coverage").GetProperty("partitions") + .EnumerateArray().Select(p => p.GetString()!).ToArray(); + + /// + /// The measured shape: the query names space B, the base is space A. It used to be answered + /// inside A, with A's coverage. No answer can honour both, so it is refused, naming both. + /// + [Fact(Timeout = 60_000)] + public async Task AQueryAnchoredOutsideTheBaseIsRefused_NotReadInTheBasesPartition() + { + var answer = await Search($"namespace:{Beta} scope:descendants name:\"{Person}\"", $"@{Alpha}", + TestContext.Current.CancellationToken); + + Output.WriteLine(answer); + answer.Should().StartWith("Error:", "a query read against a partition it did not name is the defect"); + answer.Should().Contain(Alpha).And.Contain(Beta, "the refusal names both locations"); + answer.Should().NotContain("\"count\""); + } + + /// A query anchored INSIDE the base keeps its own, narrower anchor. + [Fact(Timeout = 60_000)] + public async Task AQueryAnchoredUnderTheBaseKeepsItsOwnAnchor() + { + var answer = await Search($"namespace:{Beta}/Deal name:\"{Person}\"", $"@{Beta}", + TestContext.Current.CancellationToken); + + Output.WriteLine(answer); + Paths(answer).Should().Equal(new[] { $"{Beta}/Deal/Jane" }, "the query's own namespace, not the whole base"); + Partitions(answer).Should().Equal(Beta); + } + + /// An exclusion stays an exclusion; it used to become a free-text search FOR the namespace. + [Fact(Timeout = 60_000)] + public async Task AnExclusionUnderABaseStillExcludes() + { + var answer = await Search($"-namespace:{Beta}/Archive name:\"{Person}\"", $"@{Beta}", + TestContext.Current.CancellationToken); + + Output.WriteLine(answer); + Paths(answer).Should().Equal(new[] { $"{Beta}/Deal/Jane" }, "the archived twin is excluded, the live one is not"); + } + + /// + /// A MULTI-VALUE exclusion keeps the base (review of #6837): the parser used to defer + /// -namespace:A|B with the positive alternation and clear the anchor, so the composed + /// query lost namespace:{base} and was refused as unanchored. Both list spellings. + /// + [Theory(Timeout = 60_000)] + [InlineData($"-namespace:{Beta}/Archive|{Beta}/Old")] + [InlineData($"-namespace:({Beta}/Archive OR {Beta}/Old)")] + public async Task AMultiValueExclusionUnderABaseKeepsTheBase(string exclusion) + { + var answer = await Search($"{exclusion} name:\"{Person}\"", $"@{Beta}", + TestContext.Current.CancellationToken); + + Output.WriteLine(answer); + Paths(answer).Should().Equal(new[] { $"{Beta}/Deal/Jane" }, "the archived twin is excluded and the base still anchors"); + Partitions(answer).Should().Equal(Beta); + } + + /// + /// A location named in ONE OR branch does not bound the other (review of #6837): + /// namespace:{base}/* OR nodeType:Markdown ran its right branch everywhere and returned + /// Alpha. Refused, since no answer can keep both the query and the base. + /// + [Fact(Timeout = 60_000)] + public async Task ALocationInOnlyOneOrBranchIsRefusedUnderABase() + { + var answer = await Search($"namespace:{Beta}/* OR nodeType:Markdown partitions:all", $"@{Beta}", + TestContext.Current.CancellationToken); + + Output.WriteLine(answer); + answer.Should().StartWith("Error:", "the right branch would read outside the base"); + answer.Should().NotContain($"\"{Alpha}\""); + } + + /// + /// The base's own composition keeps the query text verbatim (review of #6837): a split-and-rejoin + /// collapsed the spaces inside a quoted value and dropped a namespace: written inside one, + /// so an exact-name search ran on another name — here, on 's two nodes. + /// + [Theory(Timeout = 60_000)] + [InlineData(SpacedPerson, $"{Beta}/Spaced")] + [InlineData(ColonPerson, $"{Beta}/Colon")] + public async Task AQuotedValueIsSearchedVerbatimUnderABase(string name, string expected) + { + var answer = await Search($"namespace: name:\"{name}\"", $"@{Beta}", + TestContext.Current.CancellationToken); + + Output.WriteLine(answer); + Paths(answer).Should().Equal(expected); + } + + /// + /// The controls on the base itself: a query with no location of its own is scoped to the whole + /// subtree, and an empty namespace: (callers' "the base's children" idiom) is no location. + /// + [Fact(Timeout = 60_000)] + public async Task AQueryWithNoLocationIsScopedByTheBase() + { + var ct = TestContext.Current.CancellationToken; + + var both = await Search($"name:\"{Person}\"", $"@{Beta}", ct); + Output.WriteLine(both); + Paths(both).Should().Equal($"{Beta}/Archive/Jane", $"{Beta}/Deal/Jane"); + Partitions(both).Should().Equal(Beta); + + var elsewhere = await Search($"name:\"{Person}\"", $"@{Alpha}", ct); + JsonDocument.Parse(elsewhere).RootElement.GetProperty("count").GetInt32().Should().Be(0, "nobody of that name lives in Alpha"); + + var children = await Search("namespace:", $"@{Beta}", ct); + Output.WriteLine(children); + Paths(children).Should().Contain(new[] { $"{Beta}/Deal", $"{Beta}/Archive" }); + Paths(children).Should().OnlyContain(p => p.StartsWith(Beta + "/", StringComparison.Ordinal)); + } +}