elastic / elastic/docs-builder
Navigation 1st principles hardening
- Dominant language
- C#
- Stars
- 24
- Forks
- 44
- Avg merge
- 1d 7h
- Merged PRs (30d)
- 146
Description
The navigation core is genuinely principled — `INavigationItem` / `ILeafNavigationItem` / `INodeNavigationItem` / `IRootNavigationItem` form a clean recursive tree, `INavigationHomeProvider` buys O(1) re-homing with lazily-computed URLs, and `NavigationRenderModel` resolves the tree once into a dumb render model the templates consume without logic. The periphery is where it frays, in four recurring patterns. None of this is in scope for the current branch — tracking here so the investigation survives.
## 1. Render mode encoded as three orthogonal booleans
`NavigationRenderModel.Create` takes `isUsingNavigationDropdown`, `isPrimaryNavEnabled`, `isGlobalAssemblyBuild` — eight combinations for three real modes. Each conditional inside (`CreateRootIndex`, `CreateBackLinks`) re-derives the mode from a different pair. One mode enum would make any production-affecting flag flip obvious rather than a one-token diff.
Related: `IsUsingNavigationDropdown` is *both* an `INavigationItem` property and a `Create` parameter, and `GlobalNavigationHtmlWriter` previously hardcoded a literal that contradicted the root's own value.
## 2. ~15 hand-rolled recursive walkers
The same tree is walked by ~15 independent recursive visitors, each re-deriving its own leaf/node/hidden/crosslink policy, despite `INavigationTraversable` existing. There is already a behavioural divergence:
- `SitemapBuilder.cs:89-100` filters on `Hidden` where the rest of the pipeline uses `ExcludeFromIndexing`, so listing pages that are deliberately "hidden but indexed" are silently missing from the sitemap.
- `CodexNavigation.cs:48-54` documents why a second root over shared nodes cannot reuse `UpdateNavigationIndex` at all — mutable `NavigationIndex` on a shared node is the leak.
## 3. `docs-content` privileged in code rather than configuration
`docs-content` is special-cased in at least five places:
- `SiteNavigation.cs:59-71` — root-leaf hoisting
- `SiteNavigation.cs:224-240` — `path_prefix`-required exemption (error path invents a `bad-mapping-…` URL segment)
- `SiteNavigationFile.cs` — URI scheme default
- `AssembleSources.cs` — implicit special casing
- `HtmlWriter.cs` — indexing rule
Additionally, `SiteNavigationFile.cs:158-257` and `:259-332` are ~70 lines of copy-paste YAML converters that share the `docs-content://` implicit scheme.
## 4. Mutation-after-construction as the extension mechanism
`IAssignableChildrenNavigation`, `IAssignableIslandNavigation`, `SectionNavigation.Url`'s `internal set`, `Index = null!` / `Id = null!` sentinels, public `Parent` and `NavigationIndex` setters — each forces a defensive check downstream.
`SiteNavigation` implements `IAssignableChildrenNavigation` and then throws from `SetNavigationItems` (`:184-185`) — the interface contract is a lie for one implementation.
---
## Feature-specific follow-ups (from the top-nav branch)
- [ ] **Aliased section URLs.** `SectionNavigation.Url` = first child's URL (`SiteNavigation.cs:96-99`) is the root cause of both the `GetParents()` URL-dedupe drop and the "← Guides goes to get-started" limitation. A synthetic section landing page gives the node its own URL and removes both special cases.
- [ ] **Sections matched by display title.** `SectionTopNavBuilder.cs:34-41` keys plain tocs by `Identifier` but sections by `Title` with `OrdinalIgnoreCase` — renaming a section in YAML silently drops its tab. Match on identity (`section://…`).
- [ ] **Dead code.** `TopNavDropdownItem` and `TopNavLinkItem.SectionIds` are never constructed anywhere in `src/` — only in tests. The dropdown branch at `_SecondaryNav.cshtml`, `secondary-nav.ts`, and `secondary-nav-dropdown.css` are unreachable in real builds. Either wire a `dropdown:` concept into the schema or delete all of it.
- [ ] **`SiteNavigation.TopLevelItems` is marked `//TODO Obsolete?`** yet is an input to `SectionTopNavBuilder` and `GlobalNavigationHtmlWriter`. Resolve before adding a third consumer.
- [ ] **String-namespace type test.** `LlmsNavigationEnhancer.cs:96` dispatches on `item.GetType().FullName?.StartsWith("Elastic.ApiExplorer.")` to avoid an assembly reference — reflection-shaped dispatch in an AOT codebase. A marker interface on `INavigationItem` removes it.
---
## Localized messes worth their own cleanups
- `GlobalNavigationPathProvider.OutputFile` (`:59-111`) — a dozen hardcoded repo/path prefixes under a `//TODO clean up`
- `ConfigurationFileProvider.CreateNavigationFile` (`:140-212`) — regex line-surgery on navigation.yml plus an inlined hardcoded `docs-builder://` block
Contributor guide
Assessment
This issue has not been assessed yet.