Skip to content

Conversation

@Mpdreamz
Copy link
Member

@Mpdreamz Mpdreamz commented Oct 3, 2025

This PR fundamentally rewrites the navigation system to solve long-standing maintainability issues and enable future extensibility.

The Problem

The original navigation implementation had four critical issues:

  1. Type erasure everywhere - All navigation returned INavigationItem<INavigationModel, INavigationItem>, forcing runtime type checks and casts throughout the codebase to access model-specific properties
  2. Tight coupling to MarkdownFile - Navigation properties (URLs, paths, breadcrumbs) duplicated on MarkdownFile itself, creating tight coupling between navigation logic and a single model implementation tied to HTML rendering
  3. Mixed concerns - Configuration parsing mixed with navigation construction, making isolated testing impossible
  4. Unclear architecture - No documentation of core principles, making contributions difficult and bugs easy to introduce

These issues blocked API documentation integration, made the assembler process opaque, and created a brittle system where navigation concerns leaked into model classes.

The Solution

1. Generic Type System with Covariance

Before:

INodeNavigationItem<INavigationModel, INavigationItem> node = GetNode();
if (node.Model is MarkdownFile markdown)  // Runtime check on every access
{
    var content = markdown.Content;
}

After:

INodeNavigationItem<MarkdownFile, INavigationItem> node = QueryForMarkdownNodes();
var content = node.Model.Content;  // Static typing, zero casts

Made all navigation classes generic over TModel where TModel : IDocumentationFile. Enables:

  • Static type safety - Query for specific types without runtime casts
  • No runtime type checks - Access model properties directly
  • Compile-time errors - Type mismatches caught during compilation
  • Extensibility - Easy to add new documentation types (API docs, generated docs)

2. Navigation as Single Source of Truth

Before: URLs, paths, and navigation properties duplicated on MarkdownFile:

public class MarkdownFile : IDocumentationFile
{
    public string Url { get; set; }                    // ❌ Duplicated
    public string Path { get; set; }                   // ❌ Duplicated
    public List<Breadcrumb> Breadcrumbs { get; set; }  // ❌ Duplicated
    // Navigation logic leaking into model class
}

After: Navigation owns all navigational concerns:

public class MarkdownFile : IDocumentationFile
{
    // Only model data, no navigation
    public string Content { get; set; }
    public FrontMatter FrontMatter { get; set; }
}

// Navigation provides URLs, paths, breadcrumbs
var url = navigationItem.Url;
var path = navigationItem.Path;

Why this matters:

  • Single authority - Navigation is the sole source of truth for URLs and paths
  • Decoupling - Models independent of navigation structure
  • Reusability - API docs, generated docs use same navigation without inheriting MarkdownFile properties
  • HTML rendering independent - Navigation not tied to specific rendering implementation

3. Dedicated Navigation Assembly

Created Elastic.Documentation.Navigation as standalone assembly:

  • Reusable across documentation types (Markdown, API, generated)
  • No markdown dependencies - Works with any IDocumentationFile implementation
  • Clean boundaries - Configuration → Navigation → Rendering pipeline
  • Testable in isolation without markdown or rendering concerns

4. Two-Phase Loading (Configuration → Navigation)

Phase 1: Configuration resolution - Parse YAML, resolve paths, validate files
Phase 2: Navigation construction - Build tree, calculate URLs, set relationships

Why this matters:

  • Separation of concerns - Configuration parsing independent of tree structure
  • Testability - Mock file system vs mock configuration
  • Reusability - Same configuration builds isolated OR assembler navigation
  • Clear errors - Know immediately if issue is configuration or structure

5. Home Provider Architecture: O(1) Re-homing

The assembler must combine repositories with custom URL prefixes. Naive approach requires O(n) tree traversal.

Our solution: Provider pattern with indirection.

// Isolated build
docset.HomeProvider = docsetNavigation;
// URLs: /api/rest/

// Assembler build - single line updates entire subtree!
docset.HomeProvider = new NavigationHomeProvider("/elasticsearch", siteNav);
// URLs: /elasticsearch/api/rest/

Time complexity: O(1) - Single reference assignment, regardless of subtree size. URLs lazy-calculated on-demand from provider. This is what makes the assembler practical at scale.

What Changed

Core Navigation (src/Elastic.Documentation.Navigation/)

  • New dedicated assembly - 13 files implementing generic navigation system
  • All navigation classes generic: DocumentationSetNavigation<TModel>
  • Navigation owns URLs, paths, breadcrumbs
  • Factory pattern ensures correct model types
  • Smart caching with automatic invalidation

Configuration (src/Elastic.Documentation.Configuration/)

  • Clean separation from navigation logic
  • Path resolution happens once during configuration
  • All paths relative to docset root after Phase 1

Assembler (src/Elastic.Documentation.Assembler/)

  • AssemblerBuild flag controls scope creation
  • O(1) re-homing via provider replacement

Models (Breaking Change)

  • MarkdownFile no longer has URL, Path, Breadcrumbs properties
  • Navigation properties accessed through navigation tree
  • Models now purely model data, no navigation concerns

Documentation (docs/development/navigation/)

10 comprehensive documents with progressive learning path:

  • Visual walkthroughs with SVG diagrams
  • Functional principles (what/why)
  • Technical principles (how)
  • Deep dives into two-phase loading, home provider architecture, assembler process
  • Complete node type reference

Why This Was Necessary

Enabling Future Work

This refactor was extensive because we needed to:

  1. Enable API documentation - Requires dedicated assembly and static typing without runtime casts
  2. Remove tight coupling - MarkdownFile shouldn't own navigation properties; blocks non-markdown docs
  3. Improve maintainability - Generic types make intent explicit, reducing bugs
  4. Document architecture - New developers can understand principles without reverse-engineering
  5. Support testing - Phase separation enables isolated unit tests
  6. Scale assembler - O(1) re-homing works for sites with thousands of pages

What This Enables

  • API documentation uses same navigation assembly with ApiDocFile model
  • Non-markdown docs (tutorials, generated content) integrate cleanly
  • Testing mocks configuration without touching file system
  • Navigation logic independent of HTML rendering
  • Future types integrate via generic type system

Backward Compatibility

All 111 tests pass. Breaking change: Models no longer have navigation properties - access through navigation tree instead.

Testing

  • All existing tests updated and passing
  • New tests for generic type system
  • Phase 1/Phase 2 separation enables cleaner test isolation
  • Mock configuration objects replace file system mocks

@Mpdreamz Mpdreamz changed the title [Draft] Build navigation from scratch Navigation Refactor: Generic Type System, O(1) Re-homing, and improved assembly routines Nov 3, 2025
@Mpdreamz Mpdreamz marked this pull request as ready for review November 3, 2025 13:24
@Mpdreamz Mpdreamz requested review from a team as code owners November 3, 2025 13:24
@Mpdreamz Mpdreamz requested a review from cotti November 3, 2025 13:24
Copy link
Contributor

@theletterf theletterf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow! Lots of changes. I guess this will enable interesting things in the future, like unified nav menus or accessing nav metadata for other features without having to reconstruct it from other bits.

My only pressing question: Does this break any existing nav definition, including crosslinks?

@Mpdreamz
Copy link
Member Author

Mpdreamz commented Nov 3, 2025

Does this break any existing nav definition, including crosslinks?

it doesn't now :) @theletterf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants