From d82473ea69c8a4713d3f6d5260f5dc43b6907971 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Thu, 2 Jul 2026 21:16:13 +0200 Subject: [PATCH] Search: guard navigation_* rank features against the contract's penalty default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit navigation_depth and navigation_table_of_contents are rank_feature fields with NEGATIVE score impact; the shared contract defaults missing values to 50, which is a penalty. Add regression tests asserting CommonEnrichments always assigns explicit, non-default values across every navigation shape it's called with (root, node, leaf, and the null-navigation OpenAPI path). Writing these tests surfaced a real bug: the API navigation_section override checked doc.Type == "api", but DocumentationDocument.Type is a fixed CLR/$type discriminator that always returns "docs" — OpenApiDocumentExporter actually sets ContentType = "api". The override could never fire. Fixed to check doc.ContentType instead. CommonEnrichments is changed from private to internal (with InternalsVisibleTo already granted to Elastic.Markdown.Tests) so it can be exercised directly without spinning up a full markdown build. Co-Authored-By: Claude Sonnet 5 --- .../ElasticsearchMarkdownExporter.Export.cs | 7 +- .../Search/NavigationEnrichmentTests.cs | 150 ++++++++++++++++++ 2 files changed, 155 insertions(+), 2 deletions(-) create mode 100644 tests/Elastic.Markdown.Tests/Search/NavigationEnrichmentTests.cs diff --git a/src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.Export.cs b/src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.Export.cs index 03b09f5517..d46892c4b8 100644 --- a/src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.Export.cs +++ b/src/Elastic.Markdown/Exporters/Elasticsearch/ElasticsearchMarkdownExporter.Export.cs @@ -44,7 +44,8 @@ private void AssignDocumentMetadata(DocumentationDocument doc) private static void AssignContentHash(DocumentationDocument doc) => doc.ContentBodyHash = ContentHash.CreateNormalized(doc.StrippedBody ?? string.Empty); - private static void CommonEnrichments(DocumentationDocument doc, INavigationItem? navigationItem) + /// Internal (rather than private) so tests can assert navigation_* rank features never fall back to the contract's penalty default. + internal static void CommonEnrichments(DocumentationDocument doc, INavigationItem? navigationItem) { doc.SearchTitle = CreateSearchTitle(); // if we have no navigation, initialize to 20 since rank_feature would score 0 too high @@ -59,7 +60,9 @@ private static void CommonEnrichments(DocumentationDocument doc, INavigationItem _ => 100 }; doc.NavigationSection = navigationItem?.NavigationSection; - if (doc.Type == "api") + // doc.Type is the fixed CLR/$type discriminator ("docs" for every DocumentationDocument) — + // ContentType is the field OpenApiDocumentExporter actually sets to "api" per-document. + if (doc.ContentType == "api") doc.NavigationSection = "api"; // this section gets promoted in the navigation we don't want it to be promoted in the search results diff --git a/tests/Elastic.Markdown.Tests/Search/NavigationEnrichmentTests.cs b/tests/Elastic.Markdown.Tests/Search/NavigationEnrichmentTests.cs new file mode 100644 index 0000000000..b8f5e431ee --- /dev/null +++ b/tests/Elastic.Markdown.Tests/Search/NavigationEnrichmentTests.cs @@ -0,0 +1,150 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using AwesomeAssertions; +using Elastic.Documentation.Navigation; +using Elastic.Documentation.Search; +using Elastic.Markdown.Exporters.Elasticsearch; + +namespace Elastic.Markdown.Tests.Search; + +/// +/// `navigation_depth` and `navigation_table_of_contents` are `rank_feature` fields with NEGATIVE +/// score impact — the shared contract defaults missing values to 50, which is a penalty. These +/// tests guard that never leaves a +/// docs page relying on that default, across every navigation shape it can be called with +/// (root, node, leaf, and the OpenAPI/no-navigation path). +/// +public class NavigationEnrichmentTests +{ + private const int PenaltyDefault = 50; + + private static DocumentationDocument NewDoc() => new() + { + Url = "/docs/reference/some-page", + Title = "Some Page", + SearchTitle = "Some Page" + }; + + [Fact] + public void LandingPageRoot_GetsLowDepthAndNonDefaultToc() + { + var root = new FakeRootNavigationItem { NavigationTitle = "Reference" }; + var doc = NewDoc(); + + ElasticsearchMarkdownExporter.CommonEnrichments(doc, root); + + doc.NavigationDepth.Should().Be(0); + doc.NavigationDepth.Should().NotBe(PenaltyDefault); + doc.NavigationTableOfContents.Should().NotBe(PenaltyDefault); + doc.NavigationSection.Should().Be("reference"); + } + + [Fact] + public void ReleaseNotesRoot_IsDampenedRelativeToOtherRootsAtTheSameDepth() + { + var releaseNotesRoot = new FakeRootNavigationItem { NavigationTitle = "Release Notes", Parent = new FakeNodeNavigationItem { NavigationTitle = "Parent" } }; + var releaseNotesDoc = NewDoc(); + ElasticsearchMarkdownExporter.CommonEnrichments(releaseNotesDoc, releaseNotesRoot); + + var otherRoot = new FakeRootNavigationItem { NavigationTitle = "Reference", Parent = new FakeNodeNavigationItem { NavigationTitle = "Parent" } }; + var otherDoc = NewDoc(); + ElasticsearchMarkdownExporter.CommonEnrichments(otherDoc, otherRoot); + + releaseNotesDoc.NavigationSection.Should().Be("release notes"); + releaseNotesDoc.NavigationTableOfContents.Should().NotBe(PenaltyDefault); + // navigation_table_of_contents has NEGATIVE score impact, so a HIGHER value means MORE + // penalty. Release notes get effectively flattened by product, so its ranking is dampened + // via a steeper multiplier (4x vs 2x depth, both capped at 48) than a regular root. + releaseNotesDoc.NavigationTableOfContents.Should().BeGreaterThan(otherDoc.NavigationTableOfContents); + } + + [Fact] + public void SectionNode_GetsFixedNonDefaultToc() + { + var root = new FakeRootNavigationItem { NavigationTitle = "Reference" }; + var node = new FakeNodeNavigationItem { NavigationTitle = "Elasticsearch", Parent = root }; + var doc = NewDoc(); + + ElasticsearchMarkdownExporter.CommonEnrichments(doc, node); + + doc.NavigationDepth.Should().BeGreaterThan(0); + doc.NavigationDepth.Should().NotBe(PenaltyDefault); + doc.NavigationTableOfContents.Should().Be(50); + } + + [Fact] + public void DeepLeafPage_GetsStructuralDepthAndNonDefaultToc() + { + var root = new FakeRootNavigationItem { NavigationTitle = "Reference" }; + var section = new FakeNodeNavigationItem { NavigationTitle = "Elasticsearch", Parent = root }; + var leaf = new FakeLeafNavigationItem { NavigationTitle = "Settings", Parent = section }; + var doc = NewDoc(); + + ElasticsearchMarkdownExporter.CommonEnrichments(doc, leaf); + + doc.NavigationDepth.Should().Be(2); + doc.NavigationDepth.Should().NotBe(PenaltyDefault); + doc.NavigationTableOfContents.Should().Be(100); + doc.NavigationSection.Should().Be("elasticsearch"); + } + + [Fact] + public void NoNavigation_OpenApiPath_FallsBackToExplicitNonPenaltyValues() + { + var doc = NewDoc(); + + ElasticsearchMarkdownExporter.CommonEnrichments(doc, null); + + // explicit fallback (20), not the contract's penalty default (50) + doc.NavigationDepth.Should().Be(20); + doc.NavigationDepth.Should().NotBe(PenaltyDefault); + doc.NavigationTableOfContents.Should().Be(100); + doc.NavigationTableOfContents.Should().NotBe(PenaltyDefault); + } + + [Fact] + public void ApiContentType_OverridesNavigationSectionRegardlessOfNavigation() + { + var root = new FakeRootNavigationItem { NavigationTitle = "Reference" }; + var doc = NewDoc(); + doc.ContentType = "api"; + + ElasticsearchMarkdownExporter.CommonEnrichments(doc, root); + + doc.NavigationSection.Should().Be("api"); + } + + private sealed class FakeNavigationModel : INavigationModel; + + private abstract class FakeNavigationItemBase : INavigationItem + { + // GetParents() dedupes by Url, so each fake needs a distinct one — not a fixed constant. + public required string NavigationTitle { get; init; } + public string Url { get; init; } = Guid.NewGuid().ToString(); + public IRootNavigationItem NavigationRoot => null!; + public INodeNavigationItem? Parent { get; set; } + public bool Hidden { get; init; } + public int NavigationIndex { get; set; } + } + + private sealed class FakeLeafNavigationItem : FakeNavigationItemBase, ILeafNavigationItem + { + public INavigationModel Model { get; } = new FakeNavigationModel(); + } + + private class FakeNodeNavigationItem : FakeNavigationItemBase, INodeNavigationItem + { + public string Id => NavigationTitle; + public ILeafNavigationItem Index => null!; + public IReadOnlyCollection NavigationItems { get; init; } = []; + } + + private sealed class FakeRootNavigationItem : FakeNodeNavigationItem, IRootNavigationItem + { + public bool IsUsingNavigationDropdown => false; + public Uri Identifier => new("https://elastic.co/docs"); + public void SetNavigationItems(IReadOnlyCollection navigationItems) { } + } +}