From 95cecd8088cb8f66734de9c8934c592eaf5cf356 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Sun, 20 Sep 2026 09:34:42 -0700 Subject: [PATCH 1/8] Read structure from a document's own outline The structure analyzer inferred everything from how a page looked: a contents page, type size, where a heading sat. That is guesswork about a layout nobody anticipated, and it is why a manual or a technical report came out as one article -- neither carries the kind of contents page the analyzer could read. PdfPig has exposed the outline all along and nothing here read it. TryGetBookmarks gives titles, nesting depth and the page each entry opens: the document stating its own structure rather than implying it. Virtually every user manual, technical manual, report and book carries one, and no heuristic can beat it. - the reader captures the outline onto the first section, because an IngestionDocument carries no metadata of its own and the outline describes the whole file rather than any one page - the analyzer prefers it over the contents page and over type size, and falls through to both when a document has none, so magazines and newspapers are unaffected - DocumentArticle gains Depth and ParentOrdinal, so a chapter and the sections beneath it are one ordered list with the nesting recorded on each. Everything that walks a document in reading order keeps working without knowing whether it is nested at all - a division runs until the next one at its own level or shallower, so a chapter spans its own sections rather than stopping at the first of them. Deeper divisions are emitted after the ones containing them, which makes the innermost division own a page the two of them share and so keeps a chapter's text from being stored twice - whatever precedes the first entry is front matter the outline does not name; it joins the first division rather than belonging to nothing, because an element owned by no division is one whose text is never stored Design notes in docs/design/general-document-structure-engine.md. Still to come: a section knowledge-object type, so a nested division is stored as one. Today a depth-two division is stored as an article, which is right about the text and wrong about the noun. Co-Authored-By: Claude Opus 5 --- .../general-document-structure-engine.md | 87 ++++++++ .../Services/PdfIngestionDocumentReader.cs | 119 +++++++++- .../ElementMetadataKeys.cs | 13 ++ .../Knowledge/Structure/DocumentArticle.cs | 27 ++- .../Structure/DocumentOutlineEntry.cs | 38 ++++ .../Structure/TocSeededStructureAnalyzer.cs | 127 ++++++++++- .../Structure/OutlineStructureTests.cs | 203 ++++++++++++++++++ 7 files changed, 611 insertions(+), 3 deletions(-) create mode 100644 docs/design/general-document-structure-engine.md create mode 100644 src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentOutlineEntry.cs create mode 100644 tests/CrestApps.Core.Tests/Core/Ingestion/Structure/OutlineStructureTests.cs diff --git a/docs/design/general-document-structure-engine.md b/docs/design/general-document-structure-engine.md new file mode 100644 index 00000000..6ce54a09 --- /dev/null +++ b/docs/design/general-document-structure-engine.md @@ -0,0 +1,87 @@ +# A general document-structure engine + +## Why the current one is not general + +`TocSeededStructureAnalyzer` splits a document into articles in three tiers: read the table of +contents, else infer from heading type size, else one article. It is a magazine analyzer with a +safe fallback, and it says so — `KnowledgeArticleTypes` has exactly two values, `Article` and +`Advertisement`. + +Three properties stop it generalising. + +**One boundary per page.** `InferHeadingBoundaries` keeps a single `title` per page and takes the +one highest on it, and `HeadingTopBand` requires that heading to sit in the top 30%. The comment +states the assumption plainly: *"a heading halfway down it is a subheading inside one"*. That is +true of a magazine and false of a newspaper, where four to six stories share a page and every +headline after the first is absorbed into the one above it. + +**No hierarchy.** `DocumentStructure.Articles` is a flat list, and `KnowledgeObjectTypes` has no +`section`. A user manual is a tree — part, chapter, section, procedure — and all of it collapses +into one article whose sections survive only as `text` chunks. "The Methods section of that paper" +cannot be returned as a hit because no such object exists. + +**Type size is the only structural signal.** Only `PdfIngestionDocumentReader` writes +`ModalPointSize`, so a document read through Azure Document Intelligence — the reader you reach for +when the file is scanned — cannot use the heading tier at all. It gets the table of contents or one +article, nothing else. + +## What is already solved + +Reading order is not the problem. The reader segments with `DocstrumBoundingBoxes`, extracts words +with `NearestNeighbourWordExtractor`, and orders blocks with `UnsupervisedReadingOrderDetector` +under `ColumnWise` rules. A newspaper's columns already extract in the right order. Only the +splitting is wrong. + +## What is available and unused + +Two authoritative structure sources ship inside PdfPig and nothing in this repository touches them. + +**The outline (`PdfDocument.TryGetBookmarks`).** `BookmarkNode` carries `Title`, `Level`, `Children` +and `IsLeaf`; `DocumentBookmarkNode` adds `PageNumber`. That is a complete hierarchical outline with +page destinations, stated by the document rather than inferred from it. Virtually every user manual, +technical manual, technical report and most white papers carry one. No heuristic can beat it. + +**Tagged content (`Page.GetMarkedContents`).** `MarkedContentElement` carries `Tag` (the structure +role), `IsArtifact`, `ActualText`, `AlternateDescription` and `Language`. For a tagged PDF this gives +real roles instead of font-size guesses, marks running heads and folios as artifacts — replacing the +`EdgeBand` heuristic — and supplies figure alt text without a vision call. + +Azure Document Intelligence is a third: its layout model already reports paragraph roles such as +`title` and `sectionHeading`, and works on scans. `DocumentIntelligenceDocumentMapper` currently +drops that role information. + +## Proposed shape + +Replace the single analyzer with a ladder of strategies behind the existing +`IDocumentStructureAnalyzer`, each one authoritative over the next, each degrading to the one below. +The contract that nothing may fail an ingest is kept: the last rung is still one article. + +| Rung | Source | Authority | Covers | +| --- | --- | --- | --- | +| 1 | Tagged content roles | stated | accessible PDFs of any genre | +| 2 | PDF outline / bookmarks | stated | manuals, reports, books, most white papers | +| 3 | Provider roles (Document Intelligence) | stated | scans, and anything the service reads | +| 4 | Table of contents | stated, fuzzy-matched | magazines, journals | +| 5 | Heading inference | inferred | anything with type-size contrast | +| 6 | Single article | none | the answer that is never wrong | + +Rung 5 is where newspapers are won or lost, and it needs two changes: allow **several boundaries per +page** rather than one, and use the block geometry the reader already produces to order them by +column rather than by height alone. + +### Model changes + +- `DocumentStructure` becomes a tree. A node carries a title, a depth, an ordinal, a page range and + its children. An article is a depth-1 node; a manual's chapter and its procedures are depth 1 and 2. +- A `section` value joins `KnowledgeObjectTypes`. This is additive — the existing values are written + into stored rows and read back verbatim, so they are pinned, but adding one is safe. +- Genre-specific notions leave the general path. `Advertisement` and section-banner capture belong to + a magazine strategy, not to every document. + +### Keeping it honest + +Each strategy reports what it relied on, so a wrong split can be explained rather than guessed at, +and `DocumentStructure.IsInferred` grows into "which rung answered". Fixtures for each genre — +magazine, newspaper, research paper, user manual, technical report — are what make this measurable +instead of anecdotal; the current behaviour was measured against one document and silently produced +a single article for everything laid out differently. diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion.Pdf/Services/PdfIngestionDocumentReader.cs b/src/Primitives/CrestApps.Core.AI.Ingestion.Pdf/Services/PdfIngestionDocumentReader.cs index a17e5fb4..3ba90eab 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion.Pdf/Services/PdfIngestionDocumentReader.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion.Pdf/Services/PdfIngestionDocumentReader.cs @@ -1,5 +1,6 @@ using System.Security.Cryptography; using System.Text.Json; +using CrestApps.Core.AI.Ingestion.Knowledge.Structure; using Microsoft.Extensions.DataIngestion; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; @@ -10,6 +11,7 @@ using UglyToad.PdfPig.DocumentLayoutAnalysis.PageSegmenter; using UglyToad.PdfPig.DocumentLayoutAnalysis.ReadingOrderDetector; using UglyToad.PdfPig.DocumentLayoutAnalysis.WordExtractor; +using UglyToad.PdfPig.Outline; using UglyToad.PdfPig.Tokens; namespace CrestApps.Core.AI.Ingestion.Pdf.Services; @@ -161,7 +163,7 @@ public override async Task ReadAsync( /// The document identifier. /// The cancellation token. /// The document. - private static IngestionDocument ReadRawPageText(PdfDocument pdf, string identifier, CancellationToken cancellationToken) + private IngestionDocument ReadRawPageText(PdfDocument pdf, string identifier, CancellationToken cancellationToken) { var document = new IngestionDocument(identifier); @@ -190,6 +192,8 @@ private static IngestionDocument ReadRawPageText(PdfDocument pdf, string identif document.Sections.Add(section); } + CaptureOutline(pdf, document, identifier); + return document; } @@ -297,9 +301,122 @@ private IngestionDocument ReadWithLayoutAnalysis(PdfDocument pdf, string identif } } + CaptureOutline(pdf, document, identifier); + return document; } + /// + /// Records the document's own outline, when it has one. + /// + /// The opened document. + /// The document being built. + /// The document identifier, used only for logging. + /// + /// An outline is the document stating its own structure, which beats anything that can be inferred from + /// how a page looks. Manuals, reports and books carry one almost without exception; magazines and + /// newspapers rarely do, and they fall through to the signals that suit them. + /// + /// Nothing here may fail a read. A malformed outline is a document without one, which is exactly what + /// the analyzer already copes with. + /// + /// + private void CaptureOutline(PdfDocument pdf, IngestionDocument document, string identifier) + { + if (document.Sections.Count == 0) + { + return; + } + + try + { + if (!pdf.TryGetBookmarks(out var bookmarks) || bookmarks.Roots.Count == 0) + { + return; + } + + var entries = new List(); + + foreach (var root in bookmarks.Roots) + { + CollectOutline(root, entries); + } + + if (entries.Count == 0) + { + return; + } + + // The outline describes the whole file, and an IngestionDocument carries no metadata of its own, + // so the first section is where it has to live. + document.Sections[0].Metadata[ElementMetadataKeys.Outline] = entries; + } + catch (Exception ex) + { + if (_logger.IsEnabled(LogLevel.Debug)) + { + _logger.LogDebug(ex, "Could not read the outline of '{Identifier}'. The document is treated as having none.", identifier); + } + } + } + + /// + /// Flattens one outline node and its descendants into document order. + /// + /// The node. + /// The entries collected so far. + /// + /// A node that groups others without pointing anywhere itself — a part that is only a heading over its + /// chapters — takes the page of the first descendant that does point somewhere. Dropping it instead + /// would flatten its children up a level and lose the nesting the document went to the trouble of + /// stating. + /// + private static void CollectOutline(BookmarkNode node, List entries) + { + var page = node is DocumentBookmarkNode document ? document.PageNumber : FindFirstPage(node); + var title = node.Title?.Trim(); + + if (page > 0 && !string.IsNullOrEmpty(title)) + { + entries.Add(new DocumentOutlineEntry + { + Title = title, + Level = node.Level, + PageNumber = page, + }); + } + + foreach (var child in node.Children) + { + CollectOutline(child, entries); + } + } + + /// + /// Finds the first page any descendant of a node points at. + /// + /// The node. + /// The page, or zero when nothing beneath it points anywhere. + private static int FindFirstPage(BookmarkNode node) + { + foreach (var child in node.Children) + { + if (child is DocumentBookmarkNode document) + { + return document.PageNumber; + } + + var page = FindFirstPage(child); + + if (page > 0) + { + return page; + } + } + + return 0; + } + /// /// Reads the lines a page draws, unless there are so many that reading them would cost minutes. /// diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/ElementMetadataKeys.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/ElementMetadataKeys.cs index 6c850bef..8775e823 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/ElementMetadataKeys.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/ElementMetadataKeys.cs @@ -63,4 +63,17 @@ public static class ElementMetadataKeys /// The page height in user-space units, recorded on the section alongside the width. /// public const string PageHeight = "crestapps.pageHeight"; + + /// + /// The document's own outline, as an + /// of in document + /// order. + /// + /// + /// Recorded on the first section rather than per page, because it describes the whole file and + /// splitting it across the pages it points at would mean reassembling an order the document already + /// stated. An IngestionDocument carries no metadata of its own — only its sections and elements + /// do — so the first section is where a document-wide fact has to live. + /// + public const string Outline = "crestapps.outline"; } diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentArticle.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentArticle.cs index aed846a6..f50543a4 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentArticle.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentArticle.cs @@ -1,8 +1,18 @@ namespace CrestApps.Core.AI.Ingestion.Knowledge.Structure; /// -/// One article within a document. +/// One division of a document, and the divisions nested inside it. /// +/// +/// "Article" is what a magazine calls its top-level division; a manual calls it a chapter and a report calls +/// it a part. The type keeps the one name because the word is written into stored rows and read back +/// verbatim, so what it means is "a top-level division, whatever this genre calls one". +/// +/// A division nested inside another is the same type at a greater . Keeping them in one +/// ordered, flat list with a depth on each — rather than a list of children — is what lets every existing +/// consumer walk the document in reading order without knowing whether it is nested at all. +/// +/// public sealed class DocumentArticle { /// @@ -10,6 +20,21 @@ public sealed class DocumentArticle /// public int Ordinal { get; init; } + /// + /// Gets how deeply the division is nested, starting at one for a top-level division. + /// + /// + /// Everything that infers structure from a page's appearance produces depth one, because a type size + /// says something is a heading and nothing about what it is a heading inside of. Only a source that + /// states nesting — a document's outline, or tagged content — produces anything deeper. + /// + public int Depth { get; init; } = 1; + + /// + /// Gets the of the division this one sits inside, or zero when it is top level. + /// + public int ParentOrdinal { get; init; } + /// /// Gets the article's title. /// diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentOutlineEntry.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentOutlineEntry.cs new file mode 100644 index 00000000..be748494 --- /dev/null +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentOutlineEntry.cs @@ -0,0 +1,38 @@ +namespace CrestApps.Core.AI.Ingestion.Knowledge.Structure; + +/// +/// One entry of a document's own outline: a title, how deeply it is nested, and the page it opens. +/// +/// +/// An outline is the rare case of a document stating its structure rather than implying it. Every other +/// signal this library reads — type size, a contents page, where a heading sits — is inference that can be +/// wrong about a layout nobody anticipated. A manual, a report or a book that carries an outline has already +/// answered the question, so a reader that can see one captures it and the analyzer prefers it over anything +/// it would otherwise work out for itself. +/// +public sealed class DocumentOutlineEntry +{ + /// + /// Gets the title as the outline states it. + /// + public string Title { get; init; } + + /// + /// Gets how deeply the entry is nested, starting at zero for a top-level entry. + /// + /// + /// This is the depth the document declares, which is not always a tidy sequence: an outline may skip + /// from a top-level entry straight to a third-level one. Consumers normalize it rather than trusting it + /// to step by one. + /// + public int Level { get; init; } + + /// + /// Gets the one-based position in the file of the page the entry opens. + /// + /// + /// This is the position in the file, not the number printed on the page. The two differ in almost every + /// publication that has front matter. + /// + public int PageNumber { get; init; } +} diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs index 05f3bbca..cc7756d4 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs @@ -136,9 +136,27 @@ public DocumentStructure Analyze(IngestionDocument document) try { var folios = CaptureFolios(document); - var (seeds, tocPageIndex) = ReadTableOfContents(document); var labels = CaptureSectionLabels(document); + // A document that states its own structure has already answered the question. Everything below + // this point infers structure from how a page looks, and inference can be wrong about a layout + // nobody anticipated; an outline cannot. + var outlined = BuildFromOutline(ReadOutline(document), pageCount); + + if (outlined.Count > 0) + { + Stamp(document, outlined, folios, labels); + + return new DocumentStructure + { + Articles = outlined, + Folios = folios, + IsInferred = true, + }; + } + + var (seeds, tocPageIndex) = ReadTableOfContents(document); + var boundaries = seeds.Count == 0 ? [] : MatchHeadings(document, seeds, tocPageIndex); @@ -188,6 +206,113 @@ public DocumentStructure Analyze(IngestionDocument document) /// /// The ingested document. /// The titles it lists, in the order it lists them. + /// + /// Reads the outline a reader captured, when there is one. + /// + /// The ingested document. + /// The outline in document order, or an empty list. + /// + /// The outline is recorded on the first section because an IngestionDocument carries no metadata + /// of its own. A reader that cannot see an outline — anything reading a format that has none, or a + /// provider-backed reader — records nothing and the analyzer carries on to the signals it can read. + /// + private static IReadOnlyList ReadOutline(IngestionDocument document) + { + if (document.Sections.Count == 0 || !document.Sections[0].HasMetadata) + { + return []; + } + + if (!document.Sections[0].Metadata.TryGetValue(ElementMetadataKeys.Outline, out var value)) + { + return []; + } + + return value as IReadOnlyList ?? []; + } + + /// + /// Turns an outline into the divisions it describes. + /// + /// The outline entries, in document order. + /// How many pages the document has. + /// The divisions, nested, or none when the outline says nothing usable. + /// + /// A division runs until the next one at its own level or shallower, which is what makes a chapter span + /// its own sections rather than stopping at the first of them. The levels an outline declares are not + /// always a tidy sequence — an outline may step from the first level to the third — so nesting is taken + /// from the order the levels appear in rather than from their values. + /// + /// Deeper divisions are emitted after the ones containing them, which is what makes the innermost + /// division own a page the two of them share, and so keeps a chapter's text from being stored twice. + /// + /// + private static List BuildFromOutline(IReadOnlyList entries, int pageCount) + { + if (entries.Count == 0 || pageCount <= 0) + { + return []; + } + + var ordered = entries + .Where(entry => entry.PageNumber > 0 && !string.IsNullOrWhiteSpace(entry.Title)) + .Select((entry, index) => (Entry: entry, Index: index)) + .OrderBy(pair => pair.Entry.PageNumber) + .ThenBy(pair => pair.Index) + .Select(pair => pair.Entry) + .ToList(); + + if (ordered.Count == 0) + { + return []; + } + + var articles = new List(ordered.Count); + var open = new List<(int Level, int Ordinal)>(); + + for (var index = 0; index < ordered.Count; index++) + { + var entry = ordered[index]; + + while (open.Count > 0 && open[^1].Level >= entry.Level) + { + open.RemoveAt(open.Count - 1); + } + + var ordinal = index + 1; + var pageEnd = pageCount; + + for (var next = index + 1; next < ordered.Count; next++) + { + if (ordered[next].Level <= entry.Level) + { + pageEnd = Math.Max(entry.PageNumber, ordered[next].PageNumber - 1); + + break; + } + } + + // Whatever precedes the first entry is front matter the outline does not name. It belongs to the + // first division rather than to nothing, because an element owned by no division is an element + // whose text is never stored. + var pageStart = index == 0 ? 1 : entry.PageNumber; + + articles.Add(new DocumentArticle + { + Ordinal = ordinal, + Depth = open.Count + 1, + ParentOrdinal = open.Count > 0 ? open[^1].Ordinal : 0, + Title = entry.Title, + PageStart = Math.Min(pageStart, pageCount), + PageEnd = Math.Min(Math.Max(pageEnd, pageStart), pageCount), + }); + + open.Add((entry.Level, ordinal)); + } + + return articles; + } + private static (List Seeds, int PageIndex) ReadTableOfContents(IngestionDocument document) { var best = new List(); diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/OutlineStructureTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/OutlineStructureTests.cs new file mode 100644 index 00000000..d7159678 --- /dev/null +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/OutlineStructureTests.cs @@ -0,0 +1,203 @@ +using CrestApps.Core.AI.Ingestion.Knowledge.Structure; +using CrestApps.Core.AI.Ingestion.Pdf.Services; +using Microsoft.Extensions.Logging.Abstractions; +using UglyToad.PdfPig.Content; +using UglyToad.PdfPig.Fonts.Standard14Fonts; +using UglyToad.PdfPig.Outline; +using UglyToad.PdfPig.Outline.Destinations; +using UglyToad.PdfPig.Writer; + +namespace CrestApps.Core.Tests.Core.Ingestion.Structure; + +/// +/// Covers reading a document's own outline and turning it into the divisions it describes. +/// +/// +/// A manual, a report or a book states its structure in an outline. Everything else the analyzer reads — +/// a contents page, type size, where a heading sits — is inference, and the point of these tests is that the +/// stated answer is taken in preference to the inferred one, with the nesting intact. +/// +public sealed class OutlineStructureTests +{ + private const string PdfMediaType = "application/pdf"; + + [Fact] + public async Task Analyze_TakesNestingFromTheOutline() + { + // A six page manual: two chapters, each with sections beneath it. + var pdf = CreateOutlinedPdf( + pageCount: 6, + new OutlineFixture("Chapter 1", 1, 1, + new OutlineFixture("Section 1.1", 2, 2), + new OutlineFixture("Section 1.2", 2, 3)), + new OutlineFixture("Chapter 2", 1, 4, + new OutlineFixture("Section 2.1", 2, 5))); + + var structure = await AnalyzeAsync(pdf); + + Assert.True(structure.IsInferred); + Assert.Equal(5, structure.Articles.Count); + + Assert.Equal( + ["Chapter 1", "Section 1.1", "Section 1.2", "Chapter 2", "Section 2.1"], + structure.Articles.Select(article => article.Title)); + + Assert.Equal([1, 2, 2, 1, 2], structure.Articles.Select(article => article.Depth)); + } + + [Fact] + public async Task Analyze_NestsSectionsUnderTheirChapter() + { + var pdf = CreateOutlinedPdf( + pageCount: 6, + new OutlineFixture("Chapter 1", 1, 1, + new OutlineFixture("Section 1.1", 2, 2), + new OutlineFixture("Section 1.2", 2, 3)), + new OutlineFixture("Chapter 2", 1, 4, + new OutlineFixture("Section 2.1", 2, 5))); + + var structure = await AnalyzeAsync(pdf); + var byTitle = structure.Articles.ToDictionary(article => article.Title, StringComparer.Ordinal); + + Assert.Equal(0, byTitle["Chapter 1"].ParentOrdinal); + Assert.Equal(byTitle["Chapter 1"].Ordinal, byTitle["Section 1.1"].ParentOrdinal); + Assert.Equal(byTitle["Chapter 1"].Ordinal, byTitle["Section 1.2"].ParentOrdinal); + Assert.Equal(0, byTitle["Chapter 2"].ParentOrdinal); + Assert.Equal(byTitle["Chapter 2"].Ordinal, byTitle["Section 2.1"].ParentOrdinal); + } + + [Fact] + public async Task Analyze_SpansAChapterAcrossItsSections() + { + var pdf = CreateOutlinedPdf( + pageCount: 6, + new OutlineFixture("Chapter 1", 1, 1, + new OutlineFixture("Section 1.1", 2, 2), + new OutlineFixture("Section 1.2", 2, 3)), + new OutlineFixture("Chapter 2", 1, 4, + new OutlineFixture("Section 2.1", 2, 5))); + + var structure = await AnalyzeAsync(pdf); + var byTitle = structure.Articles.ToDictionary(article => article.Title, StringComparer.Ordinal); + + // A chapter runs until the next chapter, not until its own first section. + Assert.Equal(1, byTitle["Chapter 1"].PageStart); + Assert.Equal(3, byTitle["Chapter 1"].PageEnd); + + Assert.Equal(4, byTitle["Chapter 2"].PageStart); + Assert.Equal(6, byTitle["Chapter 2"].PageEnd); + + // A section runs until the next entry at its own level or shallower. + Assert.Equal(2, byTitle["Section 1.1"].PageStart); + Assert.Equal(2, byTitle["Section 1.1"].PageEnd); + Assert.Equal(3, byTitle["Section 1.2"].PageStart); + Assert.Equal(3, byTitle["Section 1.2"].PageEnd); + } + + [Fact] + public async Task Analyze_GivesAPageToItsInnermostDivision() + { + var pdf = CreateOutlinedPdf( + pageCount: 6, + new OutlineFixture("Chapter 1", 1, 1, + new OutlineFixture("Section 1.1", 2, 2), + new OutlineFixture("Section 1.2", 2, 3)), + new OutlineFixture("Chapter 2", 1, 4, + new OutlineFixture("Section 2.1", 2, 5))); + + var document = await ReadAsync(pdf); + new TocSeededStructureAnalyzer(NullLogger.Instance).Analyze(document); + + var structure = await AnalyzeAsync(pdf); + var byTitle = structure.Articles.ToDictionary(article => article.Title, StringComparer.Ordinal); + + // Page 2 belongs to both Chapter 1 and Section 1.1. Storing its text against both would store it + // twice, so the innermost division owns it. + var page2 = document.Sections.Single(section => section.PageNumber == 2); + var ordinal = Assert.IsType(page2.Metadata[CrestApps.Core.AI.Ingestion.ElementMetadataKeys.ArticleOrdinal]); + + Assert.Equal(byTitle["Section 1.1"].Ordinal, ordinal); + } + + [Fact] + public async Task Analyze_KeepsFrontMatterInTheFirstDivision() + { + // The outline says nothing about pages 1 and 2. An element owned by no division is an element whose + // text is never stored, so the first division reaches back to cover them. + var pdf = CreateOutlinedPdf( + pageCount: 5, + new OutlineFixture("Chapter 1", 1, 3)); + + var structure = await AnalyzeAsync(pdf); + var chapter = Assert.Single(structure.Articles); + + Assert.Equal(1, chapter.PageStart); + Assert.Equal(5, chapter.PageEnd); + } + + [Fact] + public async Task Analyze_FallsBackWhenThereIsNoOutline() + { + var pdf = CreatePdf(pageCount: 3, bookmarks: null); + + var structure = await AnalyzeAsync(pdf); + + // No outline and nothing else to go on is one article, which is what it was before any of this. + Assert.Single(structure.Articles); + Assert.Equal(1, structure.Articles[0].Depth); + } + + private static async Task AnalyzeAsync(byte[] pdf) + { + var document = await ReadAsync(pdf); + + return new TocSeededStructureAnalyzer(NullLogger.Instance).Analyze(document); + } + + private static async Task ReadAsync(byte[] pdf) + { + using var stream = new MemoryStream(pdf, writable: false); + + return await new PdfIngestionDocumentReader().ReadAsync(stream, "manual.pdf", PdfMediaType); + } + + private static byte[] CreateOutlinedPdf(int pageCount, params OutlineFixture[] roots) + { + return CreatePdf(pageCount, new Bookmarks([.. roots.Select(root => root.ToNode())])); + } + + private static byte[] CreatePdf(int pageCount, Bookmarks bookmarks) + { + var builder = new PdfDocumentBuilder(); + var font = builder.AddStandard14Font(Standard14Font.Helvetica); + + for (var number = 1; number <= pageCount; number++) + { + var page = builder.AddPage(PageSize.A4); + + page.AddText($"Body text for page {number}.", 10, new UglyToad.PdfPig.Core.PdfPoint(50, 700), font); + } + + if (bookmarks is not null) + { + builder.Bookmarks = bookmarks; + } + + return builder.Build(); + } + + /// + /// One outline entry to write into a fixture, and the entries beneath it. + /// + private sealed record OutlineFixture(string Title, int Level, int PageNumber, params OutlineFixture[] Children) + { + public DocumentBookmarkNode ToNode() + { + return new DocumentBookmarkNode( + Title, + Level, + new ExplicitDestination(PageNumber, ExplicitDestinationType.FitPage, new ExplicitDestinationCoordinates(null)), + [.. Children.Select(child => child.ToNode())]); + } + } +} From f1b9adc8372c2fc6236e4ee64351cfc47f27dae3 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Sun, 20 Sep 2026 09:52:39 -0700 Subject: [PATCH 2/8] Divide documents by the heading levels they state Only the PDF reader wrote anything a structure analyzer could use. Word and HTML wrote nothing at all, so every .docx and every crawled page ingested as one undivided article -- despite both stating their structure more plainly than any PDF does. A paragraph styled Heading 2 and an h2 are facts, not inferences, and they were being thrown away. One metadata key now carries that fact from every format, and one strategy divides on it without knowing which reader produced the document. - ElementMetadataKeys.HeadingLevel: a one-based level an element states - the Word reader reads it from the paragraph's outline level first and its Heading N style second. The outline level comes first because a custom style built on Heading 2 carries the level without carrying the name, and reading only the style would call that document unstructured - the HTML reader now emits the blocks a page is written in rather than one run of its text, marking h1-h6 with their level. A page with no block structure still falls back to the single run it produced before - the analyzer prefers stated levels over its contents-page and type-size tiers, which are both inference, and falls through to them unchanged Divisions from stated headings are bounded by element rather than by page, because a page carrying three headings is ordinary in a manual and in anything converted from a word processor. DocumentArticle.ElementStart records where a division opens, and stamping walks the elements once and carries the open division forward. Page-bounded divisions -- an outline entry points at a page, not a paragraph -- are untouched. A nested division is now stored as a section rather than as another article, hanging off the division containing it. Retrieval can be asked for the part rather than the whole: the Methods section of a paper is a different answer from the paper. CrestApps.Core.DataIngestion takes a reference on the ingestion package to reach the shared key. It is one more assembly in a graph that already carries the AI package everywhere the HTML reader is used, and the alternative was a second spelling of the same constant. Co-Authored-By: Claude Opus 5 --- .../Indexing/KnowledgeObjectTypes.cs | 11 + .../OpenXmlIngestionDocumentReader.cs | 65 +++++- .../ElementMetadataKeys.cs | 16 ++ .../Knowledge/KnowledgeObjectBuilder.cs | 29 ++- .../Knowledge/Structure/DocumentArticle.cs | 13 ++ .../Structure/TocSeededStructureAnalyzer.cs | 213 ++++++++++++++++++ .../CrestApps.Core.DataIngestion.csproj | 4 + .../HtmlIngestionDocumentReader.cs | 114 ++++++++++ .../Structure/StatedHeadingStructureTests.cs | 197 ++++++++++++++++ 9 files changed, 656 insertions(+), 6 deletions(-) create mode 100644 tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StatedHeadingStructureTests.cs diff --git a/src/Abstractions/CrestApps.Core.Infrastructure.Abstractions/Indexing/KnowledgeObjectTypes.cs b/src/Abstractions/CrestApps.Core.Infrastructure.Abstractions/Indexing/KnowledgeObjectTypes.cs index 37880da6..4ff9713e 100644 --- a/src/Abstractions/CrestApps.Core.Infrastructure.Abstractions/Indexing/KnowledgeObjectTypes.cs +++ b/src/Abstractions/CrestApps.Core.Infrastructure.Abstractions/Indexing/KnowledgeObjectTypes.cs @@ -33,6 +33,17 @@ public static class KnowledgeObjectTypes /// public const string Article = "article"; + /// + /// One division nested inside an article: a chapter's section, a manual's procedure, a paper's Methods. + /// + /// + /// A section exists only where a document states its own nesting, because nothing that infers structure + /// from how a page looks can tell a section from an article. It is a separate type rather than another + /// article so that retrieval can be asked for the part rather than the whole — "the Methods section of + /// that paper" is a different answer from the paper. + /// + public const string Section = "section"; + /// /// One chunk of article text. This is what every row written before typed knowledge existed is read as. /// diff --git a/src/Primitives/CrestApps.Core.AI.Documents.OpenXml/Services/OpenXmlIngestionDocumentReader.cs b/src/Primitives/CrestApps.Core.AI.Documents.OpenXml/Services/OpenXmlIngestionDocumentReader.cs index 5d029914..2422f6d1 100644 --- a/src/Primitives/CrestApps.Core.AI.Documents.OpenXml/Services/OpenXmlIngestionDocumentReader.cs +++ b/src/Primitives/CrestApps.Core.AI.Documents.OpenXml/Services/OpenXmlIngestionDocumentReader.cs @@ -1,4 +1,5 @@ using System.Text; +using CrestApps.Core.AI.Ingestion; using DocumentFormat.OpenXml.Packaging; using DocumentFormat.OpenXml.Spreadsheet; using Microsoft.Extensions.DataIngestion; @@ -100,16 +101,76 @@ private static IngestionDocumentSection ExtractWord(Stream stream, CancellationT if (!string.IsNullOrWhiteSpace(paragraphText)) { - section.Elements.Add(new IngestionDocumentParagraph(paragraphText) + var element = new IngestionDocumentParagraph(paragraphText) { Text = paragraphText, - }); + }; + + var headingLevel = GetHeadingLevel(paragraph); + + if (headingLevel > 0) + { + element.Metadata[ElementMetadataKeys.HeadingLevel] = headingLevel; + } + + section.Elements.Add(element); } } return section.Elements.Count > 0 ? section : null; } + /// + /// Reads the heading level a paragraph states, if it states one. + /// + /// The paragraph. + /// The one-based heading level, or zero when the paragraph is not a heading. + /// + /// A word processor records what a paragraph is, which is the thing every other reader in this + /// library has to infer from how the text looks. Two places say it, and both are read: the outline level + /// a paragraph carries directly, and the built-in Heading N style it is formatted with. The + /// outline level is preferred because a document may restyle its headings and keep the level, and a + /// custom style built on Heading 2 carries the level without carrying the name. + /// + private static int GetHeadingLevel(DocumentFormat.OpenXml.Wordprocessing.Paragraph paragraph) + { + var properties = paragraph.ParagraphProperties; + + if (properties is null) + { + return 0; + } + + // Word stores the outline level zero-based, where zero is the outermost heading and nine means + // body text. + var outlineLevel = properties.OutlineLevel?.Val?.Value; + + if (outlineLevel is >= 0 and < 9) + { + return outlineLevel.Value + 1; + } + + var styleId = properties.ParagraphStyleId?.Val?.Value; + + if (string.IsNullOrEmpty(styleId)) + { + return 0; + } + + // The built-in styles are "Heading1".."Heading9"; a document localized at authoring time keeps the + // identifier in English even where the name shown to the author is translated. + const string HeadingPrefix = "Heading"; + + if (!styleId.StartsWith(HeadingPrefix, StringComparison.OrdinalIgnoreCase)) + { + return 0; + } + + var suffix = styleId.AsSpan(HeadingPrefix.Length).Trim(); + + return int.TryParse(suffix, out var level) && level is > 0 and <= 9 ? level : 0; + } + private static IngestionDocumentSection ExtractExcel(Stream stream, CancellationToken cancellationToken) { using var doc = SpreadsheetDocument.Open(stream, false); diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/ElementMetadataKeys.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/ElementMetadataKeys.cs index 8775e823..76df59ee 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/ElementMetadataKeys.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/ElementMetadataKeys.cs @@ -76,4 +76,20 @@ public static class ElementMetadataKeys /// do — so the first section is where a document-wide fact has to live. /// public const string Outline = "crestapps.outline"; + + /// + /// The heading level an element states it is, as a one-based where one is the + /// outermost heading. + /// + /// + /// This is for formats that name their own headings rather than implying them with type size: a Word + /// paragraph styled Heading 2, an h2 element, a tagged PDF's H2, a layout service + /// reporting a section heading. One key for all of them is what lets a single strategy divide a + /// document without knowing which reader produced it. + /// + /// A stated level is never guessed at. A reader that cannot tell a heading from body text writes + /// nothing here, and the analyzer falls through to the signals it can read. + /// + /// + public const string HeadingLevel = "crestapps.headingLevel"; } diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/KnowledgeObjectBuilder.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/KnowledgeObjectBuilder.cs index 95c1f023..e7d26639 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/KnowledgeObjectBuilder.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/KnowledgeObjectBuilder.cs @@ -76,18 +76,39 @@ public static IReadOnlyList Build( Create(options, rootId, rootId, null, KnowledgeObjectTypes.Document, title, BuildDocumentContent(title, documentText), 0), }; - foreach (var entry in ResolveArticles(options, document, pages, title)) - { - var articleId = $"{KnowledgeObjectTypes.Article}:{fileKey}:{entry.Ordinal}"; + // A division nested inside another is stored as a section, and hangs off the division containing it + // rather than off the document. The identifier carries the type, so the two are told apart by what + // they are rather than by where they happen to sit. + var divisions = ResolveArticles(options, document, pages, title); + var idsByOrdinal = divisions.ToDictionary( + division => division.Ordinal, + division => $"{(division.Depth > 1 ? KnowledgeObjectTypes.Section : KnowledgeObjectTypes.Article)}:{fileKey}:{division.Ordinal}", + EqualityComparer.Default); + + foreach (var entry in divisions) + { + var isSection = entry.Depth > 1; + var articleId = idsByOrdinal[entry.Ordinal]; var articleSegments = segments.Where(segment => segment.ArticleOrdinal == entry.Ordinal).ToList(); var articleText = string.Join('\n', articleSegments.Select(segment => segment.Text)); var articleTitle = ResolveArticleTitle(entry.Title, title, document); + var parentId = entry.ParentOrdinal > 0 && idsByOrdinal.TryGetValue(entry.ParentOrdinal, out var ancestor) + ? ancestor + : rootId; // An advertisement is kept so the document stays complete and excluded so it can never be // returned as an answer to a question about the document's subject. var isExcluded = entry.Type == KnowledgeArticleTypes.Advertisement; - var article = Create(options, articleId, rootId, rootId, KnowledgeObjectTypes.Article, articleTitle, BuildArticleContent(articleTitle, articleText), entry.Ordinal); + var article = Create( + options, + articleId, + rootId, + parentId, + isSection ? KnowledgeObjectTypes.Section : KnowledgeObjectTypes.Article, + articleTitle, + BuildArticleContent(articleTitle, articleText), + entry.Ordinal); article.PageStart = entry.PageStart > 0 ? entry.PageStart : null; article.PageEnd = entry.PageEnd > 0 ? entry.PageEnd : null; diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentArticle.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentArticle.cs index f50543a4..fef7f435 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentArticle.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentArticle.cs @@ -35,6 +35,19 @@ public sealed class DocumentArticle /// public int ParentOrdinal { get; init; } + /// + /// Gets the position, in the document's elements read in order, where this division begins, or -1 when + /// the division is bounded by pages instead. + /// + /// + /// A page is too coarse for a document that puts three headings on one of them, which is ordinary in a + /// manual and in anything converted from a word processor. Where the source states exactly which element + /// opens a division, that position is kept and the division is bounded by it; where it only states a + /// page — an outline entry points at a page, not a paragraph — this stays -1 and + /// does the work. + /// + public int ElementStart { get; init; } = -1; + /// /// Gets the article's title. /// diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs index cc7756d4..9fc2ce1f 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs @@ -155,6 +155,23 @@ public DocumentStructure Analyze(IngestionDocument document) }; } + // A format that names its own headings — a Word style, an h2, a tagged PDF, a layout service — + // states the nesting outright. That outranks a contents page, which has to be matched to the + // headings it lists before it says anything. + var headed = BuildFromHeadingLevels(document, pageCount); + + if (headed.Count > 0) + { + Stamp(document, headed, folios, labels); + + return new DocumentStructure + { + Articles = headed, + Folios = folios, + IsInferred = true, + }; + } + var (seeds, tocPageIndex) = ReadTableOfContents(document); var boundaries = seeds.Count == 0 @@ -313,6 +330,131 @@ private static List BuildFromOutline(IReadOnlyList + /// How many stated headings a document needs before they are taken as divisions. + /// + /// + /// One heading is a title. Dividing a document at its title produces one division covering all of it, + /// which is the answer the analyzer already gives when it finds nothing. + /// + private const int MinimumStatedHeadings = 2; + + /// + /// Divides a document by the heading levels its elements state. + /// + /// The ingested document. + /// How many pages the document has. + /// The divisions, nested, or none when nothing states a heading level. + /// + /// Unlike every signal below it, this reads a level the document declared rather than one worked out + /// from type size and position. A Word paragraph styled Heading 2 is a second-level heading + /// because the document says so, and the same is true of an h2, a tagged PDF's H2 and a + /// layout service's section heading. Whichever reader produced them, the levels mean the same thing. + /// + /// Divisions are bounded by element rather than by page, so three headings on one page produce three + /// divisions. A page-bounded answer would merge them and store the page's text under whichever came + /// last. + /// + /// + private static List BuildFromHeadingLevels(IngestionDocument document, int pageCount) + { + if (pageCount <= 0) + { + return []; + } + + var headings = new List<(int Level, string Title, int Page, int ElementIndex)>(); + var elementIndex = 0; + + for (var sectionIndex = 0; sectionIndex < document.Sections.Count; sectionIndex++) + { + var section = document.Sections[sectionIndex]; + var page = GetPageNumber(document, sectionIndex); + + foreach (var element in section.Elements) + { + var level = GetHeadingLevel(element); + var text = level > 0 ? CollapseHeading(element.GetSemanticText()) : null; + + if (level > 0 && !string.IsNullOrEmpty(text)) + { + headings.Add((level, text, page, elementIndex)); + } + + elementIndex++; + } + } + + if (headings.Count < MinimumStatedHeadings) + { + return []; + } + + var articles = new List(headings.Count); + var open = new List<(int Level, int Ordinal)>(); + + for (var index = 0; index < headings.Count; index++) + { + var heading = headings[index]; + + while (open.Count > 0 && open[^1].Level >= heading.Level) + { + open.RemoveAt(open.Count - 1); + } + + var pageEnd = pageCount; + + for (var next = index + 1; next < headings.Count; next++) + { + if (headings[next].Level <= heading.Level) + { + pageEnd = Math.Max(heading.Page, headings[next].Page); + + break; + } + } + + // Whatever precedes the first heading belongs to the first division, for the same reason front + // matter does: an element owned by no division is one whose text is never stored. + var isFirst = index == 0; + + articles.Add(new DocumentArticle + { + Ordinal = index + 1, + Depth = open.Count + 1, + ParentOrdinal = open.Count > 0 ? open[^1].Ordinal : 0, + Title = heading.Title, + ElementStart = isFirst ? 0 : heading.ElementIndex, + PageStart = isFirst ? 1 : heading.Page, + PageEnd = Math.Min(Math.Max(pageEnd, heading.Page), pageCount), + }); + + open.Add((heading.Level, index + 1)); + } + + return articles; + } + + /// + /// Reads the heading level an element states, if it states one. + /// + /// The element. + /// The one-based level, or zero. + private static int GetHeadingLevel(IngestionDocumentElement element) + { + if (!element.HasMetadata || !element.Metadata.TryGetValue(ElementMetadataKeys.HeadingLevel, out var value)) + { + return 0; + } + + return value switch + { + int level when level > 0 => level, + long level when level > 0 => (int)level, + _ => 0, + }; + } + private static (List Seeds, int PageIndex) ReadTableOfContents(IngestionDocument document) { var best = new List(); @@ -933,6 +1075,15 @@ private static void Stamp( Dictionary folios, Dictionary labels) { + // A division that knows which element opens it is bounded by that element, so several divisions can + // share a page. Everything that only knows a page keeps the page-bounded behaviour. + if (articles.Exists(article => article.ElementStart >= 0)) + { + StampByElement(document, articles, folios, labels); + + return; + } + var byPage = new Dictionary(); foreach (var article in articles) @@ -972,6 +1123,68 @@ private static void Stamp( } } + /// + /// Assigns every element to the division that opens at or before it. + /// + /// The ingested document. + /// The divisions, in document order. + /// The folios by page. + /// The section labels by page. + /// + /// Walking the elements once and carrying the division forward is what gives a page with three headings + /// on it three divisions. A section keeps the division its last element belongs to, because a section's + /// own metadata describes the page and a page that spans a boundary belongs to whichever division is + /// still open at its end. + /// + private static void StampByElement( + IngestionDocument document, + List articles, + Dictionary folios, + Dictionary labels) + { + var starts = new Dictionary(); + + foreach (var article in articles) + { + if (article.ElementStart >= 0) + { + starts[article.ElementStart] = article; + } + } + + var current = articles[0]; + var elementIndex = 0; + + for (var sectionIndex = 0; sectionIndex < document.Sections.Count; sectionIndex++) + { + var page = GetPageNumber(document, sectionIndex); + var section = document.Sections[sectionIndex]; + + if (folios.TryGetValue(page, out var folio)) + { + section.Metadata[ElementMetadataKeys.Folio] = folio; + } + + if (labels.TryGetValue(page, out var label)) + { + section.Metadata[ElementMetadataKeys.SectionLabel] = label; + } + + foreach (var element in section.Elements) + { + if (starts.TryGetValue(elementIndex, out var opened)) + { + current = opened; + } + + element.Metadata[ElementMetadataKeys.ArticleOrdinal] = current.Ordinal; + elementIndex++; + } + + section.Metadata[ElementMetadataKeys.ArticleOrdinal] = current.Ordinal; + } + } + /// /// Builds the answer for a document nothing was inferred about. /// diff --git a/src/Primitives/CrestApps.Core.DataIngestion/CrestApps.Core.DataIngestion.csproj b/src/Primitives/CrestApps.Core.DataIngestion/CrestApps.Core.DataIngestion.csproj index faa027ab..b4ee8b06 100644 --- a/src/Primitives/CrestApps.Core.DataIngestion/CrestApps.Core.DataIngestion.csproj +++ b/src/Primitives/CrestApps.Core.DataIngestion/CrestApps.Core.DataIngestion.csproj @@ -17,4 +17,8 @@ + + + + diff --git a/src/Primitives/CrestApps.Core.DataIngestion/HtmlIngestionDocumentReader.cs b/src/Primitives/CrestApps.Core.DataIngestion/HtmlIngestionDocumentReader.cs index d2bc4b5e..30f538c2 100644 --- a/src/Primitives/CrestApps.Core.DataIngestion/HtmlIngestionDocumentReader.cs +++ b/src/Primitives/CrestApps.Core.DataIngestion/HtmlIngestionDocumentReader.cs @@ -2,6 +2,7 @@ using System.Text.RegularExpressions; using AngleSharp.Dom; using AngleSharp.Html.Parser; +using CrestApps.Core.AI.Ingestion; using Microsoft.Extensions.DataIngestion; namespace CrestApps.Core.DataIngestion; @@ -17,6 +18,10 @@ namespace CrestApps.Core.DataIngestion; /// public sealed partial class HtmlIngestionDocumentReader : IngestionDocumentReader { + // The elements a page is written in, as opposed to the ones it is laid out with. A div wraps; a + // paragraph, a list item or a heading says something, and each becomes one element of the document. + private const string BlockSelector = "h1, h2, h3, h4, h5, h6, p, li, blockquote, pre, figcaption, td, th, dd, dt"; + // Elements whose text content must never be indexed. Their nodes (and everything inside them) are // removed before any text is read, so executable or presentational payloads cannot leak through. private const string NonContentSelector = "script, style, noscript, template, iframe, object, embed, svg, canvas, head"; @@ -53,6 +58,34 @@ public override async Task ReadAsync( public static IngestionDocument Read(string html, string identifier) { var document = new IngestionDocument(identifier); + var blocks = ExtractBlocks(html); + + if (blocks.Count > 0) + { + var section = new IngestionDocumentSection(); + + foreach (var block in blocks) + { + var element = new IngestionDocumentParagraph(block.Text) + { + Text = block.Text, + }; + + if (block.HeadingLevel > 0) + { + element.Metadata[ElementMetadataKeys.HeadingLevel] = block.HeadingLevel; + } + + section.Elements.Add(element); + } + + document.Sections.Add(section); + + return document; + } + + // A page with no block structure at all -- a fragment, or a body of bare text nodes -- still has + // text worth reading, and losing it to a stricter walk would be a regression. var text = ExtractText(html); if (!string.IsNullOrWhiteSpace(text)) @@ -69,6 +102,87 @@ public static IngestionDocument Read(string html, string identifier) return document; } + /// + /// Splits a page into the blocks it is written in, marking the ones that are headings. + /// + /// The raw HTML. + /// The blocks in document order. + /// + /// HTML states which of its text is a heading and how deeply that heading nests, which is the thing a + /// PDF has to be interrogated about. Reading the page as one run of text threw that away and made every + /// crawled page one undivided article no matter how it was written. + /// + /// Only the blocks that carry text of their own are emitted. Walking every element would emit a section + /// wrapping a heading and the heading again, and the page's text would be stored as many times as it is + /// nested deep. + /// + /// + private static List<(string Text, int HeadingLevel)> ExtractBlocks(string html) + { + var blocks = new List<(string Text, int HeadingLevel)>(); + + if (string.IsNullOrWhiteSpace(html)) + { + return blocks; + } + + var parser = new HtmlParser(); + using var document = parser.ParseDocument(html); + + foreach (var node in document.QuerySelectorAll(NonContentSelector).ToArray()) + { + node.Remove(); + } + + var root = document.Body ?? document.DocumentElement; + + if (root is null) + { + return blocks; + } + + foreach (var element in root.QuerySelectorAll(BlockSelector)) + { + // An element that contains another block is a wrapper around it; the block itself is emitted + // when the walk reaches it. + if (element.QuerySelector(BlockSelector) is not null) + { + continue; + } + + var text = CollapseWhitespace(element.TextContent); + + if (string.IsNullOrEmpty(text)) + { + continue; + } + + blocks.Add((text, GetHeadingLevel(element.LocalName))); + } + + return blocks; + } + + /// + /// Reads the heading level a tag states. + /// + /// The tag name. + /// The one-based level, or zero when the tag is not a heading. + private static int GetHeadingLevel(string localName) + { + if (string.IsNullOrEmpty(localName) || localName.Length != 2) + { + return 0; + } + + if (localName[0] is not ('h' or 'H')) + { + return 0; + } + + return localName[1] is >= '1' and <= '6' ? localName[1] - '0' : 0; + } + /// /// Extracts the page title from the parsed <title> element. /// diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StatedHeadingStructureTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StatedHeadingStructureTests.cs new file mode 100644 index 00000000..b8492b81 --- /dev/null +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StatedHeadingStructureTests.cs @@ -0,0 +1,197 @@ +using CrestApps.Core.AI.Documents.OpenXml.Services; +using CrestApps.Core.AI.Ingestion; +using CrestApps.Core.AI.Ingestion.Knowledge.Structure; +using CrestApps.Core.DataIngestion; +using DocumentFormat.OpenXml; +using DocumentFormat.OpenXml.Packaging; +using DocumentFormat.OpenXml.Wordprocessing; +using Microsoft.Extensions.DataIngestion; +using Microsoft.Extensions.Logging.Abstractions; + +namespace CrestApps.Core.Tests.Core.Ingestion.Structure; + +/// +/// Covers dividing a document by the heading levels it states, whichever format stated them. +/// +/// +/// A Word paragraph styled Heading 2 and an h2 are the same fact written two ways. One key +/// carries both, so one strategy divides both, and these tests hold that equivalence in place. +/// +public sealed class StatedHeadingStructureTests +{ + private const string WordMediaType = "application/vnd.openxmlformats-officedocument.wordprocessingml.document"; + + [Fact] + public void Html_MarksHeadingsWithTheirLevel() + { + var document = HtmlIngestionDocumentReader.Read( + "

Guide

Intro.

Install

Steps.

", + "https://example.test/guide"); + + var levels = document.Sections + .SelectMany(section => section.Elements) + .Select(GetHeadingLevel) + .ToList(); + + Assert.Equal([1, 0, 2, 0], levels); + } + + [Fact] + public void Html_DividesAPageByItsHeadings() + { + var structure = Analyze(HtmlIngestionDocumentReader.Read( + "

Guide

Intro.

Install

Steps.

Upgrade

More.

", + "https://example.test/guide")); + + Assert.Equal(["Guide", "Install", "Upgrade"], structure.Articles.Select(article => article.Title)); + Assert.Equal([1, 2, 2], structure.Articles.Select(article => article.Depth)); + + var guide = structure.Articles[0]; + + Assert.Equal(0, guide.ParentOrdinal); + Assert.Equal(guide.Ordinal, structure.Articles[1].ParentOrdinal); + Assert.Equal(guide.Ordinal, structure.Articles[2].ParentOrdinal); + } + + [Fact] + public void Html_KeepsEachDivisionsOwnTextApart() + { + // Every heading here is on one page. A page-bounded answer would merge all three and store the whole + // page under whichever came last. + var document = HtmlIngestionDocumentReader.Read( + "

Guide

Intro.

Install

Steps.

Upgrade

More.

", + "https://example.test/guide"); + + var structure = Analyze(document); + var elements = document.Sections.SelectMany(section => section.Elements).ToList(); + var ordinals = elements.Select(element => (int)element.Metadata[ElementMetadataKeys.ArticleOrdinal]).ToList(); + + var guide = structure.Articles[0].Ordinal; + var install = structure.Articles[1].Ordinal; + var upgrade = structure.Articles[2].Ordinal; + + // Guide, Intro. | Install, Steps. | Upgrade, More. + Assert.Equal([guide, guide, install, install, upgrade, upgrade], ordinals); + } + + [Fact] + public async Task Word_DividesByHeadingStyles() + { + var docx = CreateWord( + ("Manual", "Heading1"), + ("Welcome to the manual.", null), + ("Getting started", "Heading2"), + ("Plug it in.", null), + ("Troubleshooting", "Heading2"), + ("Turn it off and on.", null)); + + var structure = Analyze(await ReadWordAsync(docx)); + + Assert.Equal( + ["Manual", "Getting started", "Troubleshooting"], + structure.Articles.Select(article => article.Title)); + + Assert.Equal([1, 2, 2], structure.Articles.Select(article => article.Depth)); + } + + [Fact] + public async Task Word_ReadsTheOutlineLevelWhenAStyleIsCustom() + { + // A custom style built on Heading 2 carries the level without carrying the name. Reading only the + // style identifier would treat this document as having no headings at all. + var docx = CreateWord( + ("House Style Title", "MyBigTitle", 0), + ("Body.", null, -1), + ("House Style Section", "MySection", 1), + ("More body.", null, -1)); + + var structure = Analyze(await ReadWordAsync(docx)); + + Assert.Equal(["House Style Title", "House Style Section"], structure.Articles.Select(article => article.Title)); + Assert.Equal([1, 2], structure.Articles.Select(article => article.Depth)); + } + + [Fact] + public async Task Word_WithoutHeadingsStaysOneDivision() + { + var docx = CreateWord( + ("Just some text.", null), + ("And some more.", null)); + + var structure = Analyze(await ReadWordAsync(docx)); + + Assert.Single(structure.Articles); + } + + [Fact] + public void Html_WithASingleHeadingStaysOneDivision() + { + // One heading is a title. Dividing at it produces one division covering everything, which is the + // answer a document with no structure already gets. + var structure = Analyze(HtmlIngestionDocumentReader.Read( + "

Only a title

Body.

", + "https://example.test/one")); + + Assert.Single(structure.Articles); + } + + private static DocumentStructure Analyze(IngestionDocument document) + { + return new TocSeededStructureAnalyzer(NullLogger.Instance).Analyze(document); + } + + private static int GetHeadingLevel(IngestionDocumentElement element) + { + return element.HasMetadata && element.Metadata.TryGetValue(ElementMetadataKeys.HeadingLevel, out var value) + ? (int)value + : 0; + } + + private static async Task ReadWordAsync(byte[] docx) + { + using var stream = new MemoryStream(docx, writable: false); + + return await new OpenXmlIngestionDocumentReader().ReadAsync(stream, "manual.docx", WordMediaType); + } + + private static byte[] CreateWord(params (string Text, string StyleId)[] paragraphs) + { + return CreateWord([.. paragraphs.Select(paragraph => (paragraph.Text, paragraph.StyleId, -1))]); + } + + private static byte[] CreateWord(params (string Text, string StyleId, int OutlineLevel)[] paragraphs) + { + using var buffer = new MemoryStream(); + + using (var document = WordprocessingDocument.Create(buffer, WordprocessingDocumentType.Document, autoSave: true)) + { + var body = document.AddMainDocumentPart().Document = new Document(new Body()); + + foreach (var (text, styleId, outlineLevel) in paragraphs) + { + var paragraph = new Paragraph(new Run(new Text(text))); + + if (styleId is not null || outlineLevel >= 0) + { + var properties = new ParagraphProperties(); + + if (styleId is not null) + { + properties.ParagraphStyleId = new ParagraphStyleId { Val = styleId }; + } + + if (outlineLevel >= 0) + { + properties.OutlineLevel = new OutlineLevel { Val = outlineLevel }; + } + + paragraph.ParagraphProperties = properties; + } + + body.Body.AppendChild(paragraph); + } + } + + return buffer.ToArray(); + } +} From 4d10a9170d23f2c382c5c24eefae2a5b4468246d Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Sun, 20 Sep 2026 09:59:31 -0700 Subject: [PATCH 3/8] Read tagged PDFs, provider roles, and pages carrying several stories Three sources of structure that were being ignored, and the page shape that could not be represented at all. A tagged PDF names which of its text is a heading and at what rank. Most government and much corporate output is tagged, because publishing under an accessibility policy requires it. The reader now collects H1-H6 from the page's marked content and matches them to the blocks layout analysis produced by the text they carry -- two views of a page joined on what they say rather than on bounds that were never meant to agree. A bare H is taken as the outermost rank, which is what it is in the documents that use it. Azure Document Intelligence already reported a paragraph's role and the mapper dropped it. Title and SectionHeading now become heading levels one and two. This matters most for scans: a page of pixels states no type size, so before this nothing the analyzer reads survived one, and a scanned report divided into nothing no matter how plainly it was written. A page carrying more than one story could not be represented. The inference kept a single heading per page -- the topmost -- so every story after the first on a newspaper page was absorbed into the one above it. It now takes as many headings as a page carries, in reading order, which is already column-wise. The guard that made the old rule safe is kept where it earns its place: the first heading on a page still has to sit near the top, so a pull quote halfway down a magazine feature opens nothing. Headings after the first are taken wherever they fall, because every headline after the first on a newspaper page is below the fold. The existing magazine tests pass unchanged, which is the evidence that the narrower rule was only ever needed for the first. Articles opened this way are bounded by element, so several can share a page without the page's text going to whichever opened last. Rewriting an article keeps where it opens unless its range moved, because an element-bounded article with no opening element would hand its text to whichever article was still open. Co-Authored-By: Claude Opus 5 --- .../Services/PdfIngestionDocumentReader.cs | 116 +++++++++++++++- .../Structure/TocSeededStructureAnalyzer.cs | 50 ++++--- .../DocumentIntelligenceDocumentMapper.cs | 6 + .../Structure/MultiStoryPageTests.cs | 127 ++++++++++++++++++ 4 files changed, 280 insertions(+), 19 deletions(-) create mode 100644 tests/CrestApps.Core.Tests/Core/Ingestion/Structure/MultiStoryPageTests.cs diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion.Pdf/Services/PdfIngestionDocumentReader.cs b/src/Primitives/CrestApps.Core.AI.Ingestion.Pdf/Services/PdfIngestionDocumentReader.cs index 3ba90eab..4e7d9912 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion.Pdf/Services/PdfIngestionDocumentReader.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion.Pdf/Services/PdfIngestionDocumentReader.cs @@ -250,9 +250,11 @@ private IngestionDocument ReadWithLayoutAnalysis(PdfDocument pdf, string identif section.Metadata[ElementMetadataKeys.PageWidth] = page.Width; section.Metadata[ElementMetadataKeys.PageHeight] = page.Height; + var taggedHeadings = ReadTaggedHeadings(page, identifier); + foreach (var block in pages[pageIndex]) { - var element = CreateElement(block, pageNumber, decoration.Contains(block), heights[pageIndex]); + var element = CreateElement(block, pageNumber, decoration.Contains(block), heights[pageIndex], taggedHeadings); if (element != null) { @@ -921,7 +923,110 @@ private bool IsSafeToDrop(TextBlock block) return !TextSegmentation.ContainsSentenceBoundary(text); } - private IngestionDocumentElement CreateElement(TextBlock block, int pageNumber, bool isDecoration, double pageHeight) + /// + /// Reads the headings a tagged PDF marks, keyed by the text they carry. + /// + /// The page. + /// The document identifier, used only for logging. + /// The heading level of each marked heading, or an empty map for an untagged page. + /// + /// A tagged PDF says which of its text is a heading and at what rank, which is the thing every other + /// signal in this reader has to infer. Accessible documents — anything published under a policy that + /// requires it, which is most government and much corporate output — carry these tags. + /// + /// Marked content and the blocks layout analysis produces are two different views of a page, and the + /// honest way to join them is the text they carry: a block whose text is a marked heading's text is that + /// heading. Matching on geometry would mean reconciling two sets of bounds that were never meant to + /// agree. + /// + /// + private Dictionary ReadTaggedHeadings(Page page, string identifier) + { + var headings = new Dictionary(StringComparer.Ordinal); + + try + { + foreach (var content in page.GetMarkedContents()) + { + CollectTaggedHeadings(content, headings); + } + } + catch (Exception ex) + { + // A page whose marked content cannot be read is a page without tags, which is the ordinary case. + if (_logger.IsEnabled(LogLevel.Debug)) + { + _logger.LogDebug(ex, "Could not read the marked content of '{Identifier}'. The page is treated as untagged.", identifier); + } + } + + return headings; + } + + /// + /// Collects one marked-content element and its descendants. + /// + /// The marked content. + /// The headings collected so far. + private static void CollectTaggedHeadings(MarkedContentElement content, Dictionary headings) + { + if (!content.IsArtifact) + { + var level = GetTagHeadingLevel(content.Tag); + + if (level > 0) + { + // ActualText is what the document says the content reads as, which is what a tag exists to + // provide when the glyphs themselves do not spell it. + var text = PdfTextNormalizer.Normalize( + string.IsNullOrWhiteSpace(content.ActualText) + ? string.Concat(content.Letters.Select(letter => letter.Value)) + : content.ActualText); + + if (!string.IsNullOrWhiteSpace(text)) + { + headings[text] = level; + } + } + } + + foreach (var child in content.Children) + { + CollectTaggedHeadings(child, headings); + } + } + + /// + /// Reads the heading level a structure tag names. + /// + /// The tag. + /// The one-based level, or zero when the tag is not a heading. + /// + /// H1 to H6 state their rank. A bare H is a heading whose rank the document leaves + /// to its position in the structure tree; it is taken as the outermost, which is what it is in the + /// documents that use it at all. + /// + private static int GetTagHeadingLevel(string tag) + { + if (string.IsNullOrEmpty(tag) || tag[0] is not ('h' or 'H')) + { + return 0; + } + + if (tag.Length == 1) + { + return 1; + } + + return tag.Length == 2 && tag[1] is >= '1' and <= '6' ? tag[1] - '0' : 0; + } + + private IngestionDocumentElement CreateElement( + TextBlock block, + int pageNumber, + bool isDecoration, + double pageHeight, + Dictionary taggedHeadings) { var text = PdfTextNormalizer.Normalize(block.Text); @@ -959,6 +1064,13 @@ private IngestionDocumentElement CreateElement(TextBlock block, int pageNumber, element.Metadata[ElementMetadataKeys.BoundingBox] = new[] { bounds.Left, bounds.Bottom, bounds.Right, bounds.Top }; + // A rank the document tagged outranks anything measured from type size, and it is the only structural + // signal a heading set in body-sized type ever carries. + if (!isDecoration && taggedHeadings.Count > 0 && taggedHeadings.TryGetValue(text, out var headingLevel)) + { + element.Metadata[ElementMetadataKeys.HeadingLevel] = headingLevel; + } + var modalPointSize = GetModalPointSize(block); if (modalPointSize.HasValue) diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs index 9fc2ce1f..a117aee4 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs @@ -762,17 +762,19 @@ private static List InferHeadingBoundaries(IngestionDocument document) } var gate = bodySize * HeadingSizeRatio; + var elementIndex = 0; for (var index = 0; index < document.Sections.Count; index++) { var section = document.Sections[index]; var pageHeight = GetDouble(section, ElementMetadataKeys.PageHeight) ?? 0; - - string title = null; - var highest = double.MinValue; + var page = GetPageNumber(document, index); + var onThisPage = 0; foreach (var element in section.Elements) { + var position = elementIndex++; + if (IsDecoration(element)) { continue; @@ -794,26 +796,23 @@ private static List InferHeadingBoundaries(IngestionDocument document) var top = GetTop(element); - if (top is null || (pageHeight > 0 && top.Value < pageHeight * HeadingTopBand)) + if (top is null) { continue; } - if (top.Value > highest) + // The first heading on a page has to sit near its top, which is what keeps a pull quote + // halfway down a magazine feature from opening an article. A page that has already opened + // one takes later headings wherever they fall: a newspaper page carries four or five + // stories, and every headline after the first is somewhere below the fold. + if (onThisPage == 0 && pageHeight > 0 && top.Value < pageHeight * HeadingTopBand) { - highest = top.Value; - title = text; + continue; } - } - if (title is null) - { - continue; + onThisPage++; + boundaries.Add(new Boundary(page, new TocSeed(text, null, page), position)); } - - var page = GetPageNumber(document, index); - - boundaries.Add(new Boundary(page, new TocSeed(title, null, page))); } return boundaries.Count >= MinimumInferredArticles ? boundaries : []; @@ -866,12 +865,18 @@ private static List BuildArticles( for (var index = 0; index < boundaries.Count; index++) { var boundary = boundaries[index]; - var pageEnd = index + 1 < boundaries.Count ? boundaries[index + 1].Page - 1 : pageCount; + + // Two articles can open on one page, which is ordinary in a newspaper. The one that opens first + // still ends on that page; only the last of them carries on to the next boundary's page. + var pageEnd = index + 1 < boundaries.Count + ? (boundaries[index + 1].Page == boundary.Page ? boundary.Page : boundaries[index + 1].Page - 1) + : pageCount; titled.Add(new DocumentArticle { Title = boundary.Seed.Title, Authors = boundary.Seed.Author is null ? [] : [boundary.Seed.Author], + ElementStart = boundary.ElementStart, PageStart = boundary.Page, PageEnd = Math.Max(boundary.Page, pageEnd), SectionLabel = labels.TryGetValue(boundary.Page, out var label) ? label : null, @@ -889,6 +894,10 @@ private static List BuildArticles( articles.Insert(0, new DocumentArticle { Title = document.Identifier, + + // Front matter opens at the document's first element whenever anything else is bounded by + // one, so that stamping has somewhere to start. + ElementStart = boundaries[0].ElementStart >= 0 ? 0 : -1, PageStart = 1, PageEnd = boundaries[0].Page - 1, SectionLabel = labels.TryGetValue(1, out var frontLabel) ? frontLabel : null, @@ -1053,10 +1062,17 @@ private static DocumentArticle Rewrite(DocumentArticle article, int pageStart, i return new DocumentArticle { Ordinal = ordinal, + Depth = article.Depth, + ParentOrdinal = article.ParentOrdinal, Title = article.Title, Authors = article.Authors, SectionLabel = article.SectionLabel, Type = type, + + // A rewrite moves an article's page range; it does not move where the article opens. Dropping + // this would leave an element-bounded article with no opening element, and stamping would give + // its text to whichever article was still open. + ElementStart = pageStart == article.PageStart ? article.ElementStart : -1, PageStart = pageStart, PageEnd = pageEnd, }; @@ -1375,5 +1391,5 @@ private static double Distance(string left, string right) private readonly record struct TocSeed(string Title, string Author, int PrintedPage); - private readonly record struct Boundary(int Page, TocSeed Seed); + private readonly record struct Boundary(int Page, TocSeed Seed, int ElementStart = -1); } diff --git a/src/Primitives/CrestApps.Core.Azure.DocumentIntelligence/Services/DocumentIntelligenceDocumentMapper.cs b/src/Primitives/CrestApps.Core.Azure.DocumentIntelligence/Services/DocumentIntelligenceDocumentMapper.cs index 01c7b7bb..164e7e89 100644 --- a/src/Primitives/CrestApps.Core.Azure.DocumentIntelligence/Services/DocumentIntelligenceDocumentMapper.cs +++ b/src/Primitives/CrestApps.Core.Azure.DocumentIntelligence/Services/DocumentIntelligenceDocumentMapper.cs @@ -206,6 +206,12 @@ private static void AddParagraphs( if (role == ParagraphRole.Title || role == ParagraphRole.SectionHeading) { element.Metadata[ElementMetadataKeys.SectionLabel] = paragraph.Content; + + // The service names two ranks and no more: the thing the document is called, and a heading + // within it. Recording them as levels one and two is what lets a scanned document be divided + // at all -- nothing else the analyzer reads survives a scan, because a page of pixels states + // no type size for the heading tier to measure. + element.Metadata[ElementMetadataKeys.HeadingLevel] = role == ParagraphRole.Title ? 1 : 2; } if (role == ParagraphRole.PageNumber) diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/MultiStoryPageTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/MultiStoryPageTests.cs new file mode 100644 index 00000000..fdc450b2 --- /dev/null +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/MultiStoryPageTests.cs @@ -0,0 +1,127 @@ +using CrestApps.Core.AI.Ingestion.Knowledge.Structure; +using CrestApps.Core.AI.Ingestion.Pdf.Services; +using Microsoft.Extensions.Logging.Abstractions; +using UglyToad.PdfPig.Content; +using UglyToad.PdfPig.Core; +using UglyToad.PdfPig.Fonts.Standard14Fonts; +using UglyToad.PdfPig.Writer; + +namespace CrestApps.Core.Tests.Core.Ingestion.Structure; + +/// +/// Covers a page that carries more than one story, which is what a newspaper is and what the analyzer used +/// to be unable to represent. +/// +/// +/// Before this, one boundary per page was the most that could be found: the topmost headline opened an +/// article and every other story on the page was absorbed into it. The rule that replaced it keeps the old +/// guard for the first heading on a page — a pull quote halfway down a feature must not open anything — and +/// drops it for the rest, because every headline after the first on a newspaper page is below the fold. +/// +public sealed class MultiStoryPageTests +{ + private const string PdfMediaType = "application/pdf"; + private const double PageHeight = 842; + + [Fact] + public async Task Analyze_OpensAnArticleForEachHeadlineOnThePage() + { + var pdf = CreateNewsPage( + new Story("Council approves the budget", 800), + new Story("Bridge reopens after repairs", 520), + new Story("Library extends its hours", 260)); + + var structure = await AnalyzeAsync(pdf); + + Assert.Equal(3, structure.Articles.Count); + Assert.Equal( + ["Council approves the budget", "Bridge reopens after repairs", "Library extends its hours"], + structure.Articles.Select(article => article.Title)); + } + + [Fact] + public async Task Analyze_KeepsEachStorysOwnTextApart() + { + var pdf = CreateNewsPage( + new Story("Council approves the budget", 800), + new Story("Bridge reopens after repairs", 520), + new Story("Library extends its hours", 260)); + + var document = await ReadAsync(pdf); + var structure = new TocSeededStructureAnalyzer(NullLogger.Instance).Analyze(document); + + // Every story shares one page, so a page-bounded answer would file all of this under whichever + // headline came last. + var ordinals = document.Sections + .SelectMany(section => section.Elements) + .Select(element => (int)element.Metadata[CrestApps.Core.AI.Ingestion.ElementMetadataKeys.ArticleOrdinal]) + .Distinct() + .ToList(); + + Assert.Equal(structure.Articles.Count, ordinals.Count); + } + + [Fact] + public async Task Analyze_DoesNotOpenAnArticleForTypeBelowTheFoldOnItsOwn() + { + // One large line halfway down a page, with nothing above it, is a pull quote rather than a headline. + // The first heading on a page still has to sit near the top. + var pdf = CreateNewsPage(new Story("A line of large type", 400)); + + var structure = await AnalyzeAsync(pdf); + + Assert.Single(structure.Articles); + } + + private static async Task AnalyzeAsync(byte[] pdf) + { + return new TocSeededStructureAnalyzer(NullLogger.Instance).Analyze(await ReadAsync(pdf)); + } + + private static async Task ReadAsync(byte[] pdf) + { + using var stream = new MemoryStream(pdf, writable: false); + + return await new PdfIngestionDocumentReader().ReadAsync(stream, "gazette.pdf", PdfMediaType); + } + + /// + /// Builds a single page carrying several stories, each a headline over two lines of body text. + /// + /// The stories, top to bottom. + /// The PDF bytes. + private static byte[] CreateNewsPage(params Story[] stories) + { + var builder = new PdfDocumentBuilder(); + var font = builder.AddStandard14Font(Standard14Font.Helvetica); + var page = builder.AddPage(PageSize.A4); + + foreach (var story in stories) + { + page.AddText(story.Headline, 22, new PdfPoint(50, story.Top), font); + + // Two lines of body text, far enough below the headline that the segmenter keeps them apart and + // near enough each other that it keeps them together. + page.AddText( + "The council met on Tuesday evening to consider the matter in detail.", + 10, + new PdfPoint(50, story.Top - 30), + font); + + page.AddText( + "Several residents spoke, and a decision followed after a short debate.", + 10, + new PdfPoint(50, story.Top - 44), + font); + } + + return builder.Build(); + } + + /// + /// One story to place on the page. + /// + /// The headline text. + /// The vertical position of the headline, measured from the bottom of the page. + private sealed record Story(string Headline, double Top); +} From 7d049093f6d40395082fa8db442bf2c05c6e1e64 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Sun, 20 Sep 2026 10:05:10 -0700 Subject: [PATCH 4/8] Make the structure ladder explicit and say which rung answered The analyzer had grown four ways of dividing a document, tried in order, spelled out as nested conditionals that each returned early. The order was the important thing about the method and the hardest thing to see in it. The rungs are now an ordered list. Reading it top to bottom is reading the order of authority, and adding one is a line rather than another branch to thread a return through. DocumentStructure.Source reports which rung answered. DocumentStructure.IsInferred only ever said whether anything was found, which is not the same question and not the useful one: a document divided wrongly is far easier to argue with when the answer says whether it came from the document's own outline, from heading levels it stated, from its contents page, or from type size this library measured. No behaviour changes. The two rungs that were inline -- the contents page and the type-size inference -- became methods with the names the ladder calls them by, and the existing tests pass unchanged. Co-Authored-By: Claude Opus 5 --- .../general-document-structure-engine.md | 74 ++++++---- .../Knowledge/Structure/DocumentStructure.cs | 9 ++ .../Structure/DocumentStructureSources.cs | 38 ++++++ .../Structure/TocSeededStructureAnalyzer.cs | 128 ++++++++++-------- .../Structure/StructureLadderTests.cs | 111 +++++++++++++++ 5 files changed, 275 insertions(+), 85 deletions(-) create mode 100644 src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureSources.cs create mode 100644 tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StructureLadderTests.cs diff --git a/docs/design/general-document-structure-engine.md b/docs/design/general-document-structure-engine.md index 6ce54a09..68e6ec7a 100644 --- a/docs/design/general-document-structure-engine.md +++ b/docs/design/general-document-structure-engine.md @@ -50,38 +50,54 @@ Azure Document Intelligence is a third: its layout model already reports paragra `title` and `sectionHeading`, and works on scans. `DocumentIntelligenceDocumentMapper` currently drops that role information. -## Proposed shape +## The shape it took -Replace the single analyzer with a ladder of strategies behind the existing -`IDocumentStructureAnalyzer`, each one authoritative over the next, each degrading to the one below. -The contract that nothing may fail an ingest is kept: the last rung is still one article. +`IDocumentStructureAnalyzer` now runs a ladder, each rung authoritative over the next and each +degrading to the one below. The contract that nothing may fail an ingest is kept: the last rung is +still one division covering everything. -| Rung | Source | Authority | Covers | +| Rung | `DocumentStructureSources` | Authority | Covers | | --- | --- | --- | --- | -| 1 | Tagged content roles | stated | accessible PDFs of any genre | -| 2 | PDF outline / bookmarks | stated | manuals, reports, books, most white papers | -| 3 | Provider roles (Document Intelligence) | stated | scans, and anything the service reads | -| 4 | Table of contents | stated, fuzzy-matched | magazines, journals | -| 5 | Heading inference | inferred | anything with type-size contrast | -| 6 | Single article | none | the answer that is never wrong | - -Rung 5 is where newspapers are won or lost, and it needs two changes: allow **several boundaries per -page** rather than one, and use the block geometry the reader already produces to order them by -column rather than by height alone. +| 1 | `Outline` | stated | manuals, reports, books, most white papers | +| 2 | `StatedHeadings` | stated | Word, HTML, tagged PDFs, anything Document Intelligence read | +| 3 | `TableOfContents` | stated, fuzzy-matched | magazines, journals | +| 4 | `InferredHeadings` | inferred | anything with type-size contrast | +| 5 | `Whole` | none | the answer that is never wrong | + +Rung 2 collapsed what were going to be three separate rungs. A Word paragraph styled `Heading 2`, an +`h2`, a tagged PDF's `H2` and a layout service's section heading are one fact written four ways, so +`ElementMetadataKeys.HeadingLevel` carries it from every reader and one rung divides on it. Adding a +format means teaching its reader to write that key, not writing another strategy. + +### What each reader now states + +| Reader | Writes | +| --- | --- | +| PDF | the outline, and `H1`–`H6` from marked content on tagged pages | +| Word | the paragraph's outline level, else its `Heading N` style | +| HTML | `h1`–`h6`, and the blocks a page is written in rather than one run of its text | +| Document Intelligence | `Title` and `SectionHeading` as levels one and two | ### Model changes -- `DocumentStructure` becomes a tree. A node carries a title, a depth, an ordinal, a page range and - its children. An article is a depth-1 node; a manual's chapter and its procedures are depth 1 and 2. -- A `section` value joins `KnowledgeObjectTypes`. This is additive — the existing values are written - into stored rows and read back verbatim, so they are pinned, but adding one is safe. -- Genre-specific notions leave the general path. `Advertisement` and section-banner capture belong to - a magazine strategy, not to every document. - -### Keeping it honest - -Each strategy reports what it relied on, so a wrong split can be explained rather than guessed at, -and `DocumentStructure.IsInferred` grows into "which rung answered". Fixtures for each genre — -magazine, newspaper, research paper, user manual, technical report — are what make this measurable -instead of anecdotal; the current behaviour was measured against one document and silently produced -a single article for everything laid out differently. +- `DocumentArticle` carries `Depth` and `ParentOrdinal`, so a chapter and its sections are one + ordered list with the nesting recorded on each. Every consumer that walks a document in reading + order keeps working without knowing nesting exists. +- `ElementStart` bounds a division by element rather than by page, which is what lets several + divisions share a page — three headings on one page in a manual, four stories on a newspaper page. + Sources that only state a page, such as an outline, leave it unset. +- `KnowledgeObjectTypes.Section` stores a nested division as what it is, hanging off the division + containing it rather than off the document. +- `DocumentStructure.Source` reports which rung answered. + +### What is still open + +- **Genre-specific notions sit in the general path.** `KnowledgeArticleTypes.Advertisement` and + section-banner capture are magazine concepts every document is measured against. +- **The rungs are private methods, not public strategies.** The ladder is explicit and adding one is + a line, but a host cannot contribute a rung of its own. +- **Fixtures are synthetic.** Every test builds the document it reads — a bookmarked manual, a + Word file with heading styles, a page carrying three stories. That proves the algorithm and proves + nothing about a real magazine whose contents page is set as a picture, or a manual whose outline + points at the wrong pages. Measuring against real documents is what would turn this from correct + into trustworthy. diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructure.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructure.cs index 5ce97807..4f128c90 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructure.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructure.cs @@ -30,4 +30,13 @@ public sealed class DocumentStructure /// treated as one article because nothing could be. /// public bool IsInferred { get; init; } + + /// + /// Gets what the structure was worked out from. See . + /// + /// + /// says whether anything was found; this says what found it. The two are not + /// the same question, and the second is the one worth asking when a document comes out divided wrongly. + /// + public string Source { get; init; } = DocumentStructureSources.Whole; } diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureSources.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureSources.cs new file mode 100644 index 00000000..129e2fad --- /dev/null +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureSources.cs @@ -0,0 +1,38 @@ +namespace CrestApps.Core.AI.Ingestion.Knowledge.Structure; + +/// +/// What a document's structure was worked out from. +/// +/// +/// A split that comes out wrong is far easier to argue with when the answer says what it was based on. The +/// first three are the document stating its own structure and the last two are this library inferring it, +/// which is the distinction worth knowing before anyone starts tuning a threshold that was never consulted. +/// +public static class DocumentStructureSources +{ + /// + /// The document's own outline: bookmarks, with their nesting and the page each opens. + /// + public const string Outline = "outline"; + + /// + /// Heading levels the document states on its elements: a Word style, an h2, a tagged PDF's + /// H2, a layout service's section heading. + /// + public const string StatedHeadings = "statedHeadings"; + + /// + /// A table of contents inside the document, matched to the headings printed on its pages. + /// + public const string TableOfContents = "tableOfContents"; + + /// + /// Headings inferred from type size and position, because nothing stated any. + /// + public const string InferredHeadings = "inferredHeadings"; + + /// + /// Nothing could be worked out, so the document is one division covering all of it. + /// + public const string Whole = "whole"; +} diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs index a117aee4..1fdb3578 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs @@ -138,75 +138,39 @@ public DocumentStructure Analyze(IngestionDocument document) var folios = CaptureFolios(document); var labels = CaptureSectionLabels(document); - // A document that states its own structure has already answered the question. Everything below - // this point infers structure from how a page looks, and inference can be wrong about a layout - // nobody anticipated; an outline cannot. - var outlined = BuildFromOutline(ReadOutline(document), pageCount); + // The rungs in order of authority. The first three are the document stating its own structure, + // and the last infers it from how a page looks; a stated answer is never passed over for an + // inferred one. Each rung returns nothing when it has nothing to say, and the ladder ends at one + // division covering everything, which is the answer that is never wrong. + var ladder = new (string Source, Func> Divide)[] + { + (DocumentStructureSources.Outline, () => BuildFromOutline(ReadOutline(document), pageCount)), + (DocumentStructureSources.StatedHeadings, () => BuildFromHeadingLevels(document, pageCount)), + (DocumentStructureSources.TableOfContents, () => BuildFromTableOfContents(document, labels, pageCount)), + (DocumentStructureSources.InferredHeadings, () => BuildFromInferredHeadings(document, labels, pageCount)), + }; - if (outlined.Count > 0) + foreach (var rung in ladder) { - Stamp(document, outlined, folios, labels); + var articles = rung.Divide(); - return new DocumentStructure + if (articles.Count == 0) { - Articles = outlined, - Folios = folios, - IsInferred = true, - }; - } - - // A format that names its own headings — a Word style, an h2, a tagged PDF, a layout service — - // states the nesting outright. That outranks a contents page, which has to be matched to the - // headings it lists before it says anything. - var headed = BuildFromHeadingLevels(document, pageCount); + continue; + } - if (headed.Count > 0) - { - Stamp(document, headed, folios, labels); + Stamp(document, articles, folios, labels); return new DocumentStructure { - Articles = headed, + Articles = articles, Folios = folios, IsInferred = true, + Source = rung.Source, }; } - var (seeds, tocPageIndex) = ReadTableOfContents(document); - - var boundaries = seeds.Count == 0 - ? [] - : MatchHeadings(document, seeds, tocPageIndex); - - // A contents page is the answer key, not a precondition. Plenty of publications have none that - // can be read — the page is laid out so that no title and page number share a line, or what - // looks like a contents page is a parts list — and answering "one article" for a - // twenty-three page magazine is a worse answer than reading its own headings. - if (boundaries.Count == 0) - { - boundaries = InferHeadingBoundaries(document); - } - - if (boundaries.Count == 0) - { - return Single(document, pageCount, folios); - } - - var articles = BuildArticles(document, boundaries, labels, pageCount); - - if (articles.Count == 0) - { - return Single(document, pageCount, folios); - } - - Stamp(document, articles, folios, labels); - - return new DocumentStructure - { - Articles = articles, - Folios = folios, - IsInferred = true, - }; + return Single(document, pageCount, folios); } catch (Exception ex) { @@ -223,6 +187,58 @@ public DocumentStructure Analyze(IngestionDocument document) /// /// The ingested document. /// The titles it lists, in the order it lists them. + /// + /// Divides a document by the contents page it carries. + /// + /// The ingested document. + /// The section labels by page. + /// How many pages the document has. + /// The divisions, or none when there is no contents page the headings can be matched to. + /// + /// A contents page states the titles and the pages they are on, which turns "where does an article + /// begin" from a judgement into a fuzzy string match. It is still only as good as the match: a document + /// whose printed headings resemble nothing it lists yields nothing here. + /// + private static List BuildFromTableOfContents( + IngestionDocument document, + Dictionary labels, + int pageCount) + { + var (seeds, tocPageIndex) = ReadTableOfContents(document); + + if (seeds.Count == 0) + { + return []; + } + + var boundaries = MatchHeadings(document, seeds, tocPageIndex); + + return boundaries.Count == 0 ? [] : BuildArticles(document, boundaries, labels, pageCount); + } + + /// + /// Divides a document by the headings its type size implies. + /// + /// The ingested document. + /// The section labels by page. + /// How many pages the document has. + /// The divisions, or none when nothing on the page reads as a heading. + /// + /// The last rung before giving up, and the only one that guesses. A contents page is the answer key, not + /// a precondition: plenty of publications have none that can be read — the page is laid out so no title + /// and page number share a line, or what looks like one is a parts list — and answering "one article" + /// for a twenty-three page magazine is a worse answer than reading its own headings. + /// + private static List BuildFromInferredHeadings( + IngestionDocument document, + Dictionary labels, + int pageCount) + { + var boundaries = InferHeadingBoundaries(document); + + return boundaries.Count == 0 ? [] : BuildArticles(document, boundaries, labels, pageCount); + } + /// /// Reads the outline a reader captured, when there is one. /// diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StructureLadderTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StructureLadderTests.cs new file mode 100644 index 00000000..9e4f1661 --- /dev/null +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StructureLadderTests.cs @@ -0,0 +1,111 @@ +using CrestApps.Core.AI.Ingestion; +using CrestApps.Core.AI.Ingestion.Knowledge.Structure; +using CrestApps.Core.DataIngestion; +using Microsoft.Extensions.DataIngestion; +using Microsoft.Extensions.Logging.Abstractions; + +namespace CrestApps.Core.Tests.Core.Ingestion.Structure; + +/// +/// Covers which rung of the ladder answers, and that a stated answer is never passed over for an inferred +/// one. +/// +/// +/// A split that comes out wrong is far easier to argue with when the answer says what it was based on. These +/// tests hold that report honest, and hold the order of authority in place. +/// +public sealed class StructureLadderTests +{ + [Fact] + public void Analyze_ReportsStatedHeadingsWhenTheFormatNamesThem() + { + var structure = Analyze(HtmlIngestionDocumentReader.Read( + "

Guide

Intro.

Install

Steps.

", + "https://example.test/guide")); + + Assert.Equal(DocumentStructureSources.StatedHeadings, structure.Source); + Assert.True(structure.IsInferred); + } + + [Fact] + public void Analyze_ReportsWholeWhenNothingCanBeWorkedOut() + { + var document = new IngestionDocument("notes.txt"); + var section = new IngestionDocumentSection(); + + section.Elements.Add(new IngestionDocumentParagraph("Just a paragraph.") + { + Text = "Just a paragraph.", + }); + + document.Sections.Add(section); + + var structure = Analyze(document); + + Assert.Equal(DocumentStructureSources.Whole, structure.Source); + Assert.False(structure.IsInferred); + Assert.Single(structure.Articles); + } + + [Fact] + public void Analyze_PrefersAStatedHeadingOverAnInferredOne() + { + // The element that states a level is set in body-sized type, and the one that states nothing is set + // large. Reading type size would divide at the wrong one. + var document = new IngestionDocument("report.pdf"); + + document.Sections.Add(BuildSection(1, ("Chapter One", 10d, 1), ("Body text here.", 10d, 0))); + document.Sections.Add(BuildSection(2, ("A LARGE PULL QUOTE", 30d, 0), ("More body text.", 10d, 0))); + document.Sections.Add(BuildSection(3, ("Chapter Two", 10d, 1), ("Closing text.", 10d, 0))); + + var structure = Analyze(document); + + Assert.Equal(DocumentStructureSources.StatedHeadings, structure.Source); + Assert.Equal(["Chapter One", "Chapter Two"], structure.Articles.Select(article => article.Title)); + } + + [Fact] + public void Analyze_EmptyDocumentIsWhole() + { + var structure = Analyze(new IngestionDocument("empty.pdf")); + + Assert.Equal(DocumentStructureSources.Whole, structure.Source); + } + + private static IngestionDocumentSection BuildSection(int page, params (string Text, double Size, int HeadingLevel)[] elements) + { + var section = new IngestionDocumentSection + { + PageNumber = page, + }; + + section.Metadata[ElementMetadataKeys.PageHeight] = 842d; + section.Metadata[ElementMetadataKeys.PageWidth] = 595d; + + foreach (var (text, size, headingLevel) in elements) + { + var element = new IngestionDocumentParagraph(text) + { + Text = text, + PageNumber = page, + }; + + element.Metadata[ElementMetadataKeys.ModalPointSize] = size; + element.Metadata[ElementMetadataKeys.BoundingBox] = new[] { 50d, 780d, 545d, 800d }; + + if (headingLevel > 0) + { + element.Metadata[ElementMetadataKeys.HeadingLevel] = headingLevel; + } + + section.Elements.Add(element); + } + + return section; + } + + private static DocumentStructure Analyze(IngestionDocument document) + { + return new TocSeededStructureAnalyzer(NullLogger.Instance).Analyze(document); + } +} From c4518ed2cf37ed05b43387a7b63f3e7dc41c1735 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Sun, 20 Sep 2026 10:21:30 -0700 Subject: [PATCH 5/8] Open the structure ladder to strategies a host can contribute The ladder was explicit but closed: four rungs, private, reachable only by replacing the whole analyzer and losing the other three with it. A corpus with a convention nobody outside the business knows -- a form series whose first line names the section, a ledger split by its own rule -- had no way in. IDocumentStructureStrategy is that way in. The analyzer asks every registered strategy in Order and the first with something to say answers. The four built-in rungs are now strategies like any other, registered the same way, and their orders leave gaps -- 100, 200, 300, 400 -- so a host can place one between two of them without renumbering anything. A strategy that throws is logged and treated as having declined. One rung failing is not the document's fault, and the rung below it may well have an answer. The rung implementations stayed in one internal class rather than four. They share more than they differ -- reading a page number off a page, deciding whether a block is a running head, measuring how far one title is from another -- and splitting them would have meant a fifth file holding what all four need, with every change to a shared rule landing wherever it was least expected. What a caller sees is four small strategies; this is the library they call into. Advertisements left the general path on the way through. A page with no article and no running head means something in a magazine and nothing in a manual, so the split now runs on the table-of-contents rung rather than on every document that reaches BuildArticles. The advertisement tests pass unchanged, because they were always table-of-contents tests. TocSeededStructureAnalyzer is gone; it was named after one of the four things it had grown to do. It was added during 2.0.0 development and has never shipped, so this is its name rather than a rename. Also covers what was built but never asserted: that a nested division is stored as a section, that it hangs off the division containing it, and that its text does not also appear in its parent. Co-Authored-By: Claude Opus 5 --- .../general-document-structure-engine.md | 32 ++- .../Structure/BuiltInStructureStrategies.cs | 133 ++++++++++++ .../Structure/DocumentStructureAnalyzer.cs | 109 ++++++++++ ...eAnalyzer.cs => DocumentStructureRungs.cs} | 204 ++++++------------ .../Structure/IDocumentStructureStrategy.cs | 67 ++++++ .../ServiceCollectionExtensions.cs | 11 +- .../FileSources/FileSystemFileSourceTests.cs | 2 +- .../Knowledge/KnowledgeIngestionTests.cs | 2 +- .../Structure/CustomStructureStrategyTests.cs | 131 +++++++++++ .../Structure/MultiStoryPageTests.cs | 5 +- .../Structure/NestedKnowledgeObjectTests.cs | 110 ++++++++++ .../Structure/OutlineStructureTests.cs | 5 +- .../Structure/StatedHeadingStructureTests.cs | 3 +- .../Structure/StructureLadderTests.cs | 3 +- .../TableOfContentsStructureTests.cs} | 7 +- .../Support/FileSourceTestPipeline.cs | 2 +- .../Support/StructureAnalyzers.cs | 35 +++ 17 files changed, 699 insertions(+), 162 deletions(-) create mode 100644 src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/BuiltInStructureStrategies.cs create mode 100644 src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureAnalyzer.cs rename src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/{TocSeededStructureAnalyzer.cs => DocumentStructureRungs.cs} (86%) create mode 100644 src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/IDocumentStructureStrategy.cs create mode 100644 tests/CrestApps.Core.Tests/Core/Ingestion/Structure/CustomStructureStrategyTests.cs create mode 100644 tests/CrestApps.Core.Tests/Core/Ingestion/Structure/NestedKnowledgeObjectTests.cs rename tests/CrestApps.Core.Tests/Core/Ingestion/{Knowledge/TocSeededStructureAnalyzerTests.cs => Structure/TableOfContentsStructureTests.cs} (99%) create mode 100644 tests/CrestApps.Core.Tests/Support/StructureAnalyzers.cs diff --git a/docs/design/general-document-structure-engine.md b/docs/design/general-document-structure-engine.md index 68e6ec7a..c3fc8502 100644 --- a/docs/design/general-document-structure-engine.md +++ b/docs/design/general-document-structure-engine.md @@ -90,14 +90,28 @@ format means teaching its reader to write that key, not writing another strategy containing it rather than off the document. - `DocumentStructure.Source` reports which rung answered. +### The ladder is open + +`DocumentStructureAnalyzer` asks every registered `IDocumentStructureStrategy` in `Order`, so a host +adds a rung by registering one rather than by replacing the analyzer and losing the built-in four +with it. The built-in orders leave gaps — 100, 200, 300, 400 — so a rung can be placed between two +of them without renumbering anything. + +A corpus with a convention nobody outside the business knows, a form series whose first line names +the section, a ledger split by its own rule: those are strategies belonging to that host. A strategy +that throws is logged and treated as having declined, because one rung failing is not the document's +fault and the rung below it may well have an answer. + +Advertisements left the general path with this. A page carrying no article and no running head means +something in a magazine and nothing in a manual, so `SplitAdvertisements` runs only on the +table-of-contents rung rather than on every document that reaches `BuildArticles`. + ### What is still open -- **Genre-specific notions sit in the general path.** `KnowledgeArticleTypes.Advertisement` and - section-banner capture are magazine concepts every document is measured against. -- **The rungs are private methods, not public strategies.** The ladder is explicit and adding one is - a line, but a host cannot contribute a rung of its own. -- **Fixtures are synthetic.** Every test builds the document it reads — a bookmarked manual, a - Word file with heading styles, a page carrying three stories. That proves the algorithm and proves - nothing about a real magazine whose contents page is set as a picture, or a manual whose outline - points at the wrong pages. Measuring against real documents is what would turn this from correct - into trustworthy. +**Fixtures are synthetic.** Every test builds the document it reads — a bookmarked manual, a Word +file with heading styles, an HTML page, a page carrying three stories. That proves the algorithms +against documents written the way the specifications say. It proves nothing about a real magazine +whose contents page is set as a picture, a manual whose outline points at the wrong pages, a +newspaper whose headlines are set in the same size as its standfirsts, or a PDF that claims tags it +does not honour. Real documents are where the next class of failure lives, and measuring against +them is what would turn this from correct into trustworthy. diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/BuiltInStructureStrategies.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/BuiltInStructureStrategies.cs new file mode 100644 index 00000000..52aa7e9f --- /dev/null +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/BuiltInStructureStrategies.cs @@ -0,0 +1,133 @@ +namespace CrestApps.Core.AI.Ingestion.Knowledge.Structure; + +/// +/// Where the built-in strategies sit in the order of authority. +/// +/// +/// The gaps are deliberate. A host that knows something about its own corpus can place a strategy between +/// two built-in ones without renumbering them, which is the difference between contributing a rung and +/// forking the ladder. +/// +public static class DocumentStructureOrders +{ + /// The document's own outline. + public const int Outline = 100; + + /// Heading levels the document states on its elements. + public const int StatedHeadings = 200; + + /// A table of contents printed inside the document. + public const int TableOfContents = 300; + + /// Headings inferred from type size and position. + public const int InferredHeadings = 400; +} + +/// +/// Divides a document by the outline it carries. +/// +/// +/// The most authoritative rung there is, because an outline is the document stating its own structure with +/// its own nesting and the page each entry opens. A manual, a technical report or a book almost always +/// carries one; a magazine or a newspaper almost never does, and falls through. +/// +public sealed class OutlineStructureStrategy : IDocumentStructureStrategy +{ + /// + public string Source => DocumentStructureSources.Outline; + + /// + public int Order => DocumentStructureOrders.Outline; + + /// + public IReadOnlyList Divide(DocumentStructureContext context) + { + ArgumentNullException.ThrowIfNull(context); + + return DocumentStructureRungs.BuildFromOutline( + DocumentStructureRungs.ReadOutline(context.Document), + context.PageCount); + } +} + +/// +/// Divides a document by the heading levels its elements state. +/// +/// +/// A Word paragraph styled Heading 2, an h2, a tagged PDF's H2 and a layout service's +/// section heading are one fact written four ways. Every reader records it the same way, so this one rung +/// serves every format that states its headings rather than implying them. +/// +public sealed class StatedHeadingStructureStrategy : IDocumentStructureStrategy +{ + /// + public string Source => DocumentStructureSources.StatedHeadings; + + /// + public int Order => DocumentStructureOrders.StatedHeadings; + + /// + public IReadOnlyList Divide(DocumentStructureContext context) + { + ArgumentNullException.ThrowIfNull(context); + + return DocumentStructureRungs.BuildFromHeadingLevels(context.Document, context.PageCount); + } +} + +/// +/// Divides a document by a table of contents printed inside it. +/// +/// +/// A contents page states the titles and the pages they are on, which turns "where does an article begin" +/// from a judgement into a fuzzy string match. This is the magazine rung, and the only one that knows what +/// an advertisement is: a page with no article and no running head means something in a magazine and nothing +/// in a manual, so the notion is kept here rather than applied to every document. +/// +public sealed class TableOfContentsStructureStrategy : IDocumentStructureStrategy +{ + /// + public string Source => DocumentStructureSources.TableOfContents; + + /// + public int Order => DocumentStructureOrders.TableOfContents; + + /// + public IReadOnlyList Divide(DocumentStructureContext context) + { + ArgumentNullException.ThrowIfNull(context); + + return DocumentStructureRungs.BuildFromTableOfContents( + context.Document, + context.SectionLabels, + context.PageCount); + } +} + +/// +/// Divides a document by the headings its type size implies. +/// +/// +/// The last rung before giving up, and the only one that guesses. A contents page is an answer key, not a +/// precondition: plenty of publications have none that can be read, and answering "one article" for a +/// twenty-three page magazine is a worse answer than reading its own headings. +/// +public sealed class InferredHeadingStructureStrategy : IDocumentStructureStrategy +{ + /// + public string Source => DocumentStructureSources.InferredHeadings; + + /// + public int Order => DocumentStructureOrders.InferredHeadings; + + /// + public IReadOnlyList Divide(DocumentStructureContext context) + { + ArgumentNullException.ThrowIfNull(context); + + return DocumentStructureRungs.BuildFromInferredHeadings( + context.Document, + context.SectionLabels, + context.PageCount); + } +} diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureAnalyzer.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureAnalyzer.cs new file mode 100644 index 00000000..0d94b911 --- /dev/null +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureAnalyzer.cs @@ -0,0 +1,109 @@ +using Microsoft.Extensions.DataIngestion; +using Microsoft.Extensions.Logging; + +namespace CrestApps.Core.AI.Ingestion.Knowledge.Structure; + +/// +/// Works out what a document is made of by asking each registered strategy in turn. +/// +/// +/// Strategies are asked in their own order, which is one of authority: a document stating its own structure +/// is never passed over for this library inferring one. The first with something to say answers, and the +/// answer records which one it was. +/// +/// Nothing here may fail an ingest. A strategy that throws is logged and treated as having declined, and a +/// document no strategy could divide is one division covering all of it — which is exactly what it was +/// before any of this existed. +/// +/// +public sealed class DocumentStructureAnalyzer : IDocumentStructureAnalyzer +{ + private readonly IDocumentStructureStrategy[] _strategies; + private readonly ILogger _logger; + + /// + /// Initializes a new instance of the class. + /// + /// The strategies to ask, in any order. + /// The logger. + public DocumentStructureAnalyzer( + IEnumerable strategies, + ILogger logger) + { + ArgumentNullException.ThrowIfNull(strategies); + + _strategies = [.. strategies.OrderBy(strategy => strategy.Order)]; + _logger = logger; + } + + /// + public DocumentStructure Analyze(IngestionDocument document) + { + ArgumentNullException.ThrowIfNull(document); + + var pageCount = document.Sections.Count; + + if (pageCount == 0) + { + return DocumentStructureRungs.Single(document, 0); + } + + Dictionary folios; + Dictionary labels; + + try + { + folios = DocumentStructureRungs.CaptureFolios(document); + labels = DocumentStructureRungs.CaptureSectionLabels(document); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Could not read the page furniture of '{Identifier}'. The document is treated as one article.", document.Identifier); + + return DocumentStructureRungs.Single(document, pageCount); + } + + var context = new DocumentStructureContext(document, pageCount, folios, labels); + + foreach (var strategy in _strategies) + { + IReadOnlyList articles; + + try + { + articles = strategy.Divide(context); + } + catch (Exception ex) + { + // One strategy failing is not the document's fault, and the rung below it may well have an + // answer. Declining is the same outcome either way. + _logger.LogWarning( + ex, + "The '{Source}' structure strategy failed for '{Identifier}'. The next strategy is asked instead.", + strategy.Source, + document.Identifier); + + continue; + } + + if (articles is not { Count: > 0 }) + { + continue; + } + + var divided = articles as List ?? [.. articles]; + + DocumentStructureRungs.Stamp(document, divided, folios, labels); + + return new DocumentStructure + { + Articles = divided, + Folios = folios, + IsInferred = true, + Source = strategy.Source, + }; + } + + return DocumentStructureRungs.Single(document, pageCount, folios); + } +} diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureRungs.cs similarity index 86% rename from src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs rename to src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureRungs.cs index 1fdb3578..876b1a7c 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/TocSeededStructureAnalyzer.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureRungs.cs @@ -5,20 +5,18 @@ namespace CrestApps.Core.AI.Ingestion.Knowledge.Structure; /// -/// Splits a document into articles using its own table of contents as the answer key. +/// The work the built-in strategies do, and the primitives they share. /// /// -/// Deciding where one article ends and the next begins from headings alone is guesswork: a big line of type -/// might be a title, a pull quote or an advertisement. A table of contents states the answer — these are the -/// titles, in this order, on these printed pages — so the only remaining question is which page carries -/// which title, and that is a fuzzy string match rather than a judgement call. +/// This is one class rather than four because the strategies share more than they differ: reading a page +/// number off a page, deciding whether a block is a running head, measuring how far one title is from +/// another. Splitting those across four files would mean a fifth file holding what all four need, and every +/// change to a shared rule would land in whichever file it was least expected. /// -/// Every step degrades to the step before it. No table of contents, no headings big enough to be titles, or -/// anything at all going wrong, and the document is one article — which is exactly what it was before any of -/// this existed. +/// What a caller sees is the strategies, which are small and separate. This is the library they call into. /// /// -public sealed partial class TocSeededStructureAnalyzer : IDocumentStructureAnalyzer +internal static partial class DocumentStructureRungs { /// /// How many leading pages may hold the table of contents. Past that it is a list inside an article, not @@ -37,24 +35,24 @@ public sealed partial class TocSeededStructureAnalyzer : IDocumentStructureAnaly /// then match nothing falls back to one article anyway. /// /// - private const int MaxTocPageIndex = 10; + internal const int MaxTocPageIndex = 10; /// /// How many "title ... page" lines a page needs before it is believed to be a table of contents. One or /// two such lines happen by accident in ordinary prose; five do not. /// - private const int MinimumTocEntries = 5; + internal const int MinimumTocEntries = 5; /// /// How much bigger than the body a line has to be before it is treated as a heading. /// - private const double HeadingSizeRatio = 1.5; + internal const double HeadingSizeRatio = 1.5; /// /// How different a heading may be from a table-of-contents title and still be taken as the same title. /// Printed headings drop subtitles, change case and hyphenate, so an exact match finds almost nothing. /// - private const double MaxTitleDistance = 0.3; + internal const double MaxTitleDistance = 0.3; /// /// How much of what a prefix match left out still counts against it, as a fraction of that difference. @@ -68,30 +66,30 @@ public sealed partial class TocSeededStructureAnalyzer : IDocumentStructureAnaly /// is forgiven at all — and large enough that a title printed in full always beats one that merely starts /// the same way. /// - private const double PrefixPenalty = 0.5; + internal const double PrefixPenalty = 0.5; /// /// How near an edge a number has to be printed before it is taken for the page number rather than for /// part of the text, as a fraction of the page height. /// - private const double EdgeBand = 0.12; + internal const double EdgeBand = 0.12; /// /// How far down a page an article's opening heading may sit, as a fraction of the page height measured /// from the bottom. A title opens a page; a heading halfway down it is a subheading inside one. /// - private const double HeadingTopBand = 0.7; + internal const double HeadingTopBand = 0.7; /// /// How many headed pages a document needs before its headings alone are taken as article boundaries. /// Below this the document is barely divided, and one article is the safer answer. /// - private const int MinimumInferredArticles = 3; + internal const int MinimumInferredArticles = 3; /// /// The longest a heading may be and still be stored as an article title. /// - private const int MaxInferredTitleCharacters = 200; + internal const int MaxInferredTitleCharacters = 200; /// /// What share of the pages inside articles have to carry a running head before the absence of one on a @@ -108,85 +106,8 @@ public sealed partial class TocSeededStructureAnalyzer : IDocumentStructureAnaly /// matter carries no running head by convention and would otherwise drag every document under the bar. /// /// - private const double MinLabelledPageRatio = 0.6; - - private readonly ILogger _logger; + internal const double MinLabelledPageRatio = 0.6; - /// - /// Initializes a new instance of the class. - /// - /// The logger. - public TocSeededStructureAnalyzer(ILogger logger) - { - _logger = logger; - } - - /// - public DocumentStructure Analyze(IngestionDocument document) - { - ArgumentNullException.ThrowIfNull(document); - - var pageCount = document.Sections.Count; - - if (pageCount == 0) - { - return Single(document, 0); - } - - try - { - var folios = CaptureFolios(document); - var labels = CaptureSectionLabels(document); - - // The rungs in order of authority. The first three are the document stating its own structure, - // and the last infers it from how a page looks; a stated answer is never passed over for an - // inferred one. Each rung returns nothing when it has nothing to say, and the ladder ends at one - // division covering everything, which is the answer that is never wrong. - var ladder = new (string Source, Func> Divide)[] - { - (DocumentStructureSources.Outline, () => BuildFromOutline(ReadOutline(document), pageCount)), - (DocumentStructureSources.StatedHeadings, () => BuildFromHeadingLevels(document, pageCount)), - (DocumentStructureSources.TableOfContents, () => BuildFromTableOfContents(document, labels, pageCount)), - (DocumentStructureSources.InferredHeadings, () => BuildFromInferredHeadings(document, labels, pageCount)), - }; - - foreach (var rung in ladder) - { - var articles = rung.Divide(); - - if (articles.Count == 0) - { - continue; - } - - Stamp(document, articles, folios, labels); - - return new DocumentStructure - { - Articles = articles, - Folios = folios, - IsInferred = true, - Source = rung.Source, - }; - } - - return Single(document, pageCount, folios); - } - catch (Exception ex) - { - // A document that could not be understood is still a document. One article is the answer that is - // never wrong, only less useful. - _logger.LogWarning(ex, "Structure analysis failed for '{Identifier}'. The document is treated as one article.", document.Identifier); - - return Single(document, pageCount); - } - } - - /// - /// Reads the table of contents, when the document has one in its front matter. - /// - /// The ingested document. - /// The titles it lists, in the order it lists them. /// /// Divides a document by the contents page it carries. /// @@ -199,9 +120,9 @@ public DocumentStructure Analyze(IngestionDocument document) /// begin" from a judgement into a fuzzy string match. It is still only as good as the match: a document /// whose printed headings resemble nothing it lists yields nothing here. /// - private static List BuildFromTableOfContents( + internal static List BuildFromTableOfContents( IngestionDocument document, - Dictionary labels, + IReadOnlyDictionary labels, int pageCount) { var (seeds, tocPageIndex) = ReadTableOfContents(document); @@ -213,7 +134,7 @@ private static List BuildFromTableOfContents( var boundaries = MatchHeadings(document, seeds, tocPageIndex); - return boundaries.Count == 0 ? [] : BuildArticles(document, boundaries, labels, pageCount); + return boundaries.Count == 0 ? [] : BuildArticles(document, boundaries, labels, pageCount, splitAdvertisements: true); } /// @@ -229,14 +150,14 @@ private static List BuildFromTableOfContents( /// and page number share a line, or what looks like one is a parts list — and answering "one article" /// for a twenty-three page magazine is a worse answer than reading its own headings. /// - private static List BuildFromInferredHeadings( + internal static List BuildFromInferredHeadings( IngestionDocument document, - Dictionary labels, + IReadOnlyDictionary labels, int pageCount) { var boundaries = InferHeadingBoundaries(document); - return boundaries.Count == 0 ? [] : BuildArticles(document, boundaries, labels, pageCount); + return boundaries.Count == 0 ? [] : BuildArticles(document, boundaries, labels, pageCount, splitAdvertisements: false); } /// @@ -249,7 +170,7 @@ private static List BuildFromInferredHeadings( /// of its own. A reader that cannot see an outline — anything reading a format that has none, or a /// provider-backed reader — records nothing and the analyzer carries on to the signals it can read. /// - private static IReadOnlyList ReadOutline(IngestionDocument document) + internal static IReadOnlyList ReadOutline(IngestionDocument document) { if (document.Sections.Count == 0 || !document.Sections[0].HasMetadata) { @@ -280,7 +201,7 @@ private static IReadOnlyList ReadOutline(IngestionDocument /// division own a page the two of them share, and so keeps a chapter's text from being stored twice. /// /// - private static List BuildFromOutline(IReadOnlyList entries, int pageCount) + internal static List BuildFromOutline(IReadOnlyList entries, int pageCount) { if (entries.Count == 0 || pageCount <= 0) { @@ -353,7 +274,7 @@ private static List BuildFromOutline(IReadOnlyList - private const int MinimumStatedHeadings = 2; + internal const int MinimumStatedHeadings = 2; /// /// Divides a document by the heading levels its elements state. @@ -372,7 +293,7 @@ private static List BuildFromOutline(IReadOnlyList /// - private static List BuildFromHeadingLevels(IngestionDocument document, int pageCount) + internal static List BuildFromHeadingLevels(IngestionDocument document, int pageCount) { if (pageCount <= 0) { @@ -456,7 +377,7 @@ private static List BuildFromHeadingLevels(IngestionDocument do /// /// The element. /// The one-based level, or zero. - private static int GetHeadingLevel(IngestionDocumentElement element) + internal static int GetHeadingLevel(IngestionDocumentElement element) { if (!element.HasMetadata || !element.Metadata.TryGetValue(ElementMetadataKeys.HeadingLevel, out var value)) { @@ -471,7 +392,7 @@ private static int GetHeadingLevel(IngestionDocumentElement element) }; } - private static (List Seeds, int PageIndex) ReadTableOfContents(IngestionDocument document) + internal static (List Seeds, int PageIndex) ReadTableOfContents(IngestionDocument document) { var best = new List(); var bestIndex = -1; @@ -527,7 +448,7 @@ private static (List Seeds, int PageIndex) ReadTableOfContents(Ingestio /// /// The line. /// The title and the author, which may be . - private static (string Title, string Author) SplitAuthor(string line) + internal static (string Title, string Author) SplitAuthor(string line) { var separator = line.LastIndexOfAny(['–', '—']); @@ -554,7 +475,7 @@ private static (string Title, string Author) SplitAuthor(string line) /// /// The ingested document. /// The folio of each page, keyed by the page's position in the file. - private static Dictionary CaptureFolios(IngestionDocument document) + internal static Dictionary CaptureFolios(IngestionDocument document) { var folios = new Dictionary(); @@ -607,7 +528,7 @@ private static Dictionary CaptureFolios(IngestionDocument document) /// A label recurring on one page only is a title that happens to be set in capitals. Two pages is the /// smallest number that makes it a running head. /// - private static Dictionary CaptureSectionLabels(IngestionDocument document) + internal static Dictionary CaptureSectionLabels(IngestionDocument document) { var candidates = new Dictionary>(StringComparer.OrdinalIgnoreCase); @@ -668,7 +589,7 @@ private static Dictionary CaptureSectionLabels(IngestionDocument do /// whichever of the entries it resembled happened to be listed earliest, and a heading that matched one /// entry exactly was filed under another. /// - private static List MatchHeadings(IngestionDocument document, List seeds, int tocPageIndex) + internal static List MatchHeadings(IngestionDocument document, List seeds, int tocPageIndex) { var bodySize = GetBodyPointSize(document); var boundaries = new List(); @@ -767,7 +688,7 @@ private static List MatchHeadings(IngestionDocument document, List /// - private static List InferHeadingBoundaries(IngestionDocument document) + internal static List InferHeadingBoundaries(IngestionDocument document) { var boundaries = new List(); var bodySize = GetBodyPointSize(document); @@ -839,7 +760,7 @@ private static List InferHeadingBoundaries(IngestionDocument document) /// /// The element. /// The top coordinate, or when the element carries no bounds. - private static double? GetTop(IngestionDocumentElement element) + internal static double? GetTop(IngestionDocumentElement element) { if (!element.HasMetadata || !element.Metadata.TryGetValue(ElementMetadataKeys.BoundingBox, out var raw) || @@ -856,7 +777,7 @@ private static List InferHeadingBoundaries(IngestionDocument document) /// /// The heading text. /// The collapsed title, trimmed to a storable length. - private static string CollapseHeading(string text) + internal static string CollapseHeading(string text) { if (string.IsNullOrWhiteSpace(text)) { @@ -870,11 +791,12 @@ private static string CollapseHeading(string text) : collapsed[..MaxInferredTitleCharacters].TrimEnd(); } - private static List BuildArticles( + internal static List BuildArticles( IngestionDocument document, List boundaries, - Dictionary labels, - int pageCount) + IReadOnlyDictionary labels, + int pageCount, + bool splitAdvertisements) { var titled = new List(); @@ -900,7 +822,9 @@ private static List BuildArticles( }); } - var articles = SplitAdvertisements(titled, boundaries, labels, pageCount); + var articles = splitAdvertisements + ? SplitAdvertisements(titled, boundaries, labels, pageCount) + : titled; // Whatever comes before the first matched title is front matter: the cover, the contents, the // masthead. It is one article, and it is never an advertisement - a contents page carries no @@ -944,10 +868,10 @@ private static List BuildArticles( /// rule arms on the share of pages that carry a label rather than on whether any page does. /// /// - private static List SplitAdvertisements( + internal static List SplitAdvertisements( List articles, List boundaries, - Dictionary labels, + IReadOnlyDictionary labels, int pageCount) { if (!LabelsPagesConsistently(articles, labels)) @@ -1011,7 +935,7 @@ private static List SplitAdvertisements( /// /// The articles. /// The numbered articles. - private static List Number(List articles) + internal static List Number(List articles) { var ordinal = 1; @@ -1040,14 +964,14 @@ private static List Number(List articles) /// Everything downstream, a figure's page above all, carries the real page number, so keying anything by /// position silently shifts every article boundary, running head and folio after the first missing page. /// - private static int GetPageNumber(IngestionDocument document, int index) + internal static int GetPageNumber(IngestionDocument document, int index) { var pageNumber = document.Sections[index].PageNumber; return pageNumber is > 0 ? pageNumber.Value : index + 1; } - private static bool LabelsPagesConsistently(List articles, Dictionary labels) + internal static bool LabelsPagesConsistently(List articles, IReadOnlyDictionary labels) { if (labels.Count == 0) { @@ -1073,7 +997,7 @@ private static bool LabelsPagesConsistently(List articles, Dict return covered > 0 && labelled >= covered * MinLabelledPageRatio; } - private static DocumentArticle Rewrite(DocumentArticle article, int pageStart, int pageEnd, string type, int ordinal = 0) + internal static DocumentArticle Rewrite(DocumentArticle article, int pageStart, int pageEnd, string type, int ordinal = 0) { return new DocumentArticle { @@ -1101,7 +1025,7 @@ private static DocumentArticle Rewrite(DocumentArticle article, int pageStart, i /// The articles. /// The folio of each page. /// The running-head label on each page. - private static void Stamp( + internal static void Stamp( IngestionDocument document, List articles, Dictionary folios, @@ -1168,7 +1092,7 @@ private static void Stamp( /// own metadata describes the page and a page that spans a boundary belongs to whichever division is /// still open at its end. /// - private static void StampByElement( + internal static void StampByElement( IngestionDocument document, List articles, Dictionary folios, @@ -1224,7 +1148,7 @@ private static void StampByElement( /// How many pages the document has. /// The folios, when they were read before the analysis gave up. /// One article covering the whole document. - private static DocumentStructure Single(IngestionDocument document, int pageCount, Dictionary folios = null) + internal static DocumentStructure Single(IngestionDocument document, int pageCount, Dictionary folios = null) { return new DocumentStructure { @@ -1249,7 +1173,7 @@ private static DocumentStructure Single(IngestionDocument document, int pageCoun /// /// The ingested document. /// The body point size, or zero when nothing recorded one. - private static double GetBodyPointSize(IngestionDocument document) + internal static double GetBodyPointSize(IngestionDocument document) { var weights = new Dictionary(); @@ -1276,7 +1200,7 @@ private static double GetBodyPointSize(IngestionDocument document) return weights.Count == 0 ? 0 : weights.MaxBy(entry => entry.Value).Key; } - private static bool IsNearEdge(IngestionDocumentElement element, double pageHeight) + internal static bool IsNearEdge(IngestionDocumentElement element, double pageHeight) { if (!element.HasMetadata || !element.Metadata.TryGetValue(ElementMetadataKeys.BoundingBox, out var raw) || @@ -1290,12 +1214,12 @@ private static bool IsNearEdge(IngestionDocumentElement element, double pageHeig return box[1] <= band || box[3] >= pageHeight - band; } - private static bool IsDecoration(IngestionDocumentElement element) + internal static bool IsDecoration(IngestionDocumentElement element) { return IngestionDocumentElementExtensions.IsDecoration(element); } - private static double? GetDouble(IngestionDocumentElement element, string key) + internal static double? GetDouble(IngestionDocumentElement element, string key) { if (!element.HasMetadata || !element.Metadata.TryGetValue(key, out var value)) { @@ -1317,7 +1241,7 @@ private static bool IsDecoration(IngestionDocumentElement element) /// /// The title. /// The folded title. - private static string Normalize(string value) + internal static string Normalize(string value) { if (string.IsNullOrWhiteSpace(value)) { @@ -1350,7 +1274,7 @@ private static string Normalize(string value) /// two apart, at the discount, so a heading spelled out in full always /// outscores one that only begins the same way. /// - private static double Distance(string left, string right) + internal static double Distance(string left, string right) { if (left.Length == 0 || right.Length == 0) { @@ -1392,20 +1316,20 @@ private static double Distance(string left, string right) } [GeneratedRegex(@"^(.+?)[\s.·…]+(\d{1,3})$", RegexOptions.CultureInvariant)] - private static partial Regex TocLine(); + internal static partial Regex TocLine(); [GeneratedRegex(@"^\D*(\d{1,3})\D*$", RegexOptions.CultureInvariant)] - private static partial Regex FolioToken(); + internal static partial Regex FolioToken(); // Uppercase letters and spaces in any script, so this reads a Hungarian, Greek or Cyrillic running head // as readily as a Latin one. [GeneratedRegex(@"^[\p{Lu}\p{Lm}\p{Lo}][\p{Lu}\p{Lm}\p{Lo}\s'’\-]{5,}$", RegexOptions.CultureInvariant)] - private static partial Regex SectionLabel(); + internal static partial Regex SectionLabel(); [GeneratedRegex(@"\s+", RegexOptions.CultureInvariant)] - private static partial Regex WhitespaceRun(); + internal static partial Regex WhitespaceRun(); - private readonly record struct TocSeed(string Title, string Author, int PrintedPage); + internal readonly record struct TocSeed(string Title, string Author, int PrintedPage); - private readonly record struct Boundary(int Page, TocSeed Seed, int ElementStart = -1); + internal readonly record struct Boundary(int Page, TocSeed Seed, int ElementStart = -1); } diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/IDocumentStructureStrategy.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/IDocumentStructureStrategy.cs new file mode 100644 index 00000000..55f02588 --- /dev/null +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/IDocumentStructureStrategy.cs @@ -0,0 +1,67 @@ +using Microsoft.Extensions.DataIngestion; + +namespace CrestApps.Core.AI.Ingestion.Knowledge.Structure; + +/// +/// One way of working out what a document is made of. +/// +/// +/// Strategies are tried in , and the first that has something to say answers. The order +/// is one of authority rather than preference: a document stating its own structure is never passed over for +/// this library inferring one, however confident the inference. +/// +/// A host adds a strategy by registering one. A corpus with a convention of its own — a form series whose +/// first line names the section, a ledger split by a rule nobody outside the business knows — is a strategy +/// that belongs to that host and to nothing else, and it can be given an above or below +/// the built-in ones as the corpus demands. +/// +/// +public interface IDocumentStructureStrategy +{ + /// + /// Gets what this strategy divides a document by, recorded on the answer it produces. + /// See . + /// + string Source { get; } + + /// + /// Gets where this strategy sits in the order of authority. Lower runs first. + /// + /// + /// The built-in strategies leave gaps between their orders so a host can place one between them without + /// renumbering anything. + /// + int Order { get; } + + /// + /// Divides one document. + /// + /// What is known about the document so far. + /// + /// The divisions in document order, or an empty list when this strategy has nothing to say about this + /// document. + /// + /// + /// Returning nothing is how a strategy declines. Nothing here may fail an ingest: a strategy that throws + /// is logged and treated as having declined, because a document that could not be divided is still a + /// document. + /// + IReadOnlyList Divide(DocumentStructureContext context); +} + +/// +/// What is known about a document when a strategy is asked to divide it. +/// +/// The ingested document. +/// How many pages it has. +/// The page number printed on each page, keyed by position in the file. +/// The running-head label on each page, keyed by position in the file. +/// +/// Folios and labels are read once, before any strategy runs, because every strategy would otherwise read +/// them again and they say the same thing whoever asks. +/// +public sealed record DocumentStructureContext( + IngestionDocument Document, + int PageCount, + IReadOnlyDictionary Folios, + IReadOnlyDictionary SectionLabels); diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/ServiceCollectionExtensions.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/ServiceCollectionExtensions.cs index 3516162e..6ed4d17b 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/ServiceCollectionExtensions.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/ServiceCollectionExtensions.cs @@ -168,7 +168,16 @@ public static IServiceCollection AddCoreAIDocumentIngestion(this IServiceCollect // Typed knowledge: an ingested file becomes separate, individually retrievable objects in an // AI data source. services.AddOptions(); - services.TryAddSingleton(); + // The ladder, in the order of authority the strategies declare. A host adds a rung of its own by + // registering another IDocumentStructureStrategy; it does not have to replace the analyzer to do it. + services.TryAddSingleton(); + services.TryAddEnumerable( + [ + ServiceDescriptor.Singleton(), + ServiceDescriptor.Singleton(), + ServiceDescriptor.Singleton(), + ServiceDescriptor.Singleton(), + ]); services.TryAddScoped(); services.TryAddScoped(); services.TryAddScoped(); diff --git a/tests/CrestApps.Core.Tests/Core/FileSources/FileSystemFileSourceTests.cs b/tests/CrestApps.Core.Tests/Core/FileSources/FileSystemFileSourceTests.cs index 134f05ab..a4d1d0c9 100644 --- a/tests/CrestApps.Core.Tests/Core/FileSources/FileSystemFileSourceTests.cs +++ b/tests/CrestApps.Core.Tests/Core/FileSources/FileSystemFileSourceTests.cs @@ -390,7 +390,7 @@ public Harness(string root) Store, FileStore, new DefaultAITextNormalizer(), - new TocSeededStructureAnalyzer(NullLogger.Instance), + StructureAnalyzers.Default(), new NullPublicationMetadataExtractor(), Queue, NullLogger.Instance); diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Knowledge/KnowledgeIngestionTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Knowledge/KnowledgeIngestionTests.cs index 8c8307eb..5bee1521 100644 --- a/tests/CrestApps.Core.Tests/Core/Ingestion/Knowledge/KnowledgeIngestionTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Knowledge/KnowledgeIngestionTests.cs @@ -512,7 +512,7 @@ private static DefaultKnowledgeIngestionService CreateService( store, fileStore, new DefaultAITextNormalizer(), - structureAnalyzer ?? new TocSeededStructureAnalyzer(NullLogger.Instance), + structureAnalyzer ?? StructureAnalyzers.Default(), publicationMetadataExtractor ?? new NullPublicationMetadataExtractor(), indexingQueue, NullLogger.Instance); diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/CustomStructureStrategyTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/CustomStructureStrategyTests.cs new file mode 100644 index 00000000..56523c4b --- /dev/null +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/CustomStructureStrategyTests.cs @@ -0,0 +1,131 @@ +using CrestApps.Core.AI.Ingestion; +using CrestApps.Core.AI.Ingestion.Knowledge.Structure; +using CrestApps.Core.DataIngestion; +using CrestApps.Core.Tests.Support; +using Microsoft.Extensions.DataIngestion; + +namespace CrestApps.Core.Tests.Core.Ingestion.Structure; + +/// +/// Covers a host contributing a rung of its own. +/// +/// +/// A corpus with a convention nobody outside the business knows — a form series whose first line names the +/// section, a ledger split by a rule of its own — is a strategy belonging to that host. The point of the +/// ladder being open is that such a host adds a rung rather than replacing the analyzer and losing the four +/// built-in ones with it. +/// +public sealed class CustomStructureStrategyTests +{ + [Fact] + public void Analyze_UsesAHostStrategyPlacedAboveTheBuiltInOnes() + { + // The document states headings, so the built-in ladder would divide it at those. A host rung placed + // above them wins. + var document = HtmlIngestionDocumentReader.Read( + "

Guide

Intro.

Install

Steps.

", + "https://example.test/guide"); + + var structure = StructureAnalyzers + .Default(new FirstLineStrategy(DocumentStructureOrders.Outline - 1)) + .Analyze(document); + + Assert.Equal(FirstLineStrategy.SourceName, structure.Source); + Assert.Equal(["Guide"], structure.Articles.Select(article => article.Title)); + } + + [Fact] + public void Analyze_FallsThroughToAHostStrategyPlacedBelowTheBuiltInOnes() + { + // Nothing here states a heading and nothing infers one, so every built-in rung declines and the + // host's rung answers instead of the document becoming one undivided division. + var document = new IngestionDocument("ledger.txt"); + var section = new IngestionDocumentSection(); + + section.Elements.Add(new IngestionDocumentParagraph("Opening balance carried forward.") + { + Text = "Opening balance carried forward.", + }); + + document.Sections.Add(section); + + var structure = StructureAnalyzers + .Default(new FirstLineStrategy(DocumentStructureOrders.InferredHeadings + 1)) + .Analyze(document); + + Assert.Equal(FirstLineStrategy.SourceName, structure.Source); + Assert.Equal(["Opening balance carried forward."], structure.Articles.Select(article => article.Title)); + } + + [Fact] + public void Analyze_TreatsAFailingStrategyAsHavingDeclined() + { + // A rung that throws must cost the document that rung, not its ingest. + var document = HtmlIngestionDocumentReader.Read( + "

Guide

Intro.

Install

Steps.

", + "https://example.test/guide"); + + var structure = StructureAnalyzers + .Default(new ThrowingStrategy()) + .Analyze(document); + + Assert.Equal(DocumentStructureSources.StatedHeadings, structure.Source); + Assert.Equal(["Guide", "Install"], structure.Articles.Select(article => article.Title)); + } + + /// + /// A host rung that calls the document's first line its only division. + /// + private sealed class FirstLineStrategy : IDocumentStructureStrategy + { + public const string SourceName = "firstLine"; + + public FirstLineStrategy(int order) + { + Order = order; + } + + public string Source => SourceName; + + public int Order { get; } + + public IReadOnlyList Divide(DocumentStructureContext context) + { + var first = context.Document.Sections + .SelectMany(section => section.Elements) + .Select(element => element.Text) + .FirstOrDefault(text => !string.IsNullOrWhiteSpace(text)); + + if (first is null) + { + return []; + } + + return + [ + new DocumentArticle + { + Ordinal = 1, + Title = first, + PageStart = 1, + PageEnd = context.PageCount, + }, + ]; + } + } + + /// + /// A host rung that is broken. + /// + private sealed class ThrowingStrategy : IDocumentStructureStrategy + { + public string Source => "throwing"; + + public int Order => DocumentStructureOrders.Outline - 1; + + public IReadOnlyList Divide(DocumentStructureContext context) + { + throw new InvalidOperationException("This strategy is deliberately broken."); + } + } +} diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/MultiStoryPageTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/MultiStoryPageTests.cs index fdc450b2..e4a8ef21 100644 --- a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/MultiStoryPageTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/MultiStoryPageTests.cs @@ -1,5 +1,6 @@ using CrestApps.Core.AI.Ingestion.Knowledge.Structure; using CrestApps.Core.AI.Ingestion.Pdf.Services; +using CrestApps.Core.Tests.Support; using Microsoft.Extensions.Logging.Abstractions; using UglyToad.PdfPig.Content; using UglyToad.PdfPig.Core; @@ -48,7 +49,7 @@ public async Task Analyze_KeepsEachStorysOwnTextApart() new Story("Library extends its hours", 260)); var document = await ReadAsync(pdf); - var structure = new TocSeededStructureAnalyzer(NullLogger.Instance).Analyze(document); + var structure = StructureAnalyzers.Default().Analyze(document); // Every story shares one page, so a page-bounded answer would file all of this under whichever // headline came last. @@ -75,7 +76,7 @@ public async Task Analyze_DoesNotOpenAnArticleForTypeBelowTheFoldOnItsOwn() private static async Task AnalyzeAsync(byte[] pdf) { - return new TocSeededStructureAnalyzer(NullLogger.Instance).Analyze(await ReadAsync(pdf)); + return StructureAnalyzers.Default().Analyze(await ReadAsync(pdf)); } private static async Task ReadAsync(byte[] pdf) diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/NestedKnowledgeObjectTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/NestedKnowledgeObjectTests.cs new file mode 100644 index 00000000..5a79aabe --- /dev/null +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/NestedKnowledgeObjectTests.cs @@ -0,0 +1,110 @@ +using CrestApps.Core.AI.Ingestion; +using CrestApps.Core.AI.Ingestion.Knowledge; +using CrestApps.Core.AI.Ingestion.Knowledge.Structure; +using CrestApps.Core.AI.Models; +using CrestApps.Core.DataIngestion; +using CrestApps.Core.Infrastructure.Indexing; +using CrestApps.Core.Tests.Support; +using Microsoft.Extensions.DataIngestion; + +namespace CrestApps.Core.Tests.Core.Ingestion.Structure; + +/// +/// Covers what a nested division is stored as. +/// +/// +/// Working out the nesting is only half of it. Storing a section as another article would leave retrieval +/// unable to tell the part from the whole, which is the reason for computing the nesting at all. +/// +public sealed class NestedKnowledgeObjectTests +{ + [Fact] + public void Build_StoresANestedDivisionAsASection() + { + var objects = BuildGuide(); + + var article = Assert.Single(objects, entry => entry.ObjectType == KnowledgeObjectTypes.Article); + var sections = objects.Where(entry => entry.ObjectType == KnowledgeObjectTypes.Section).ToList(); + + Assert.Equal("Guide", article.Title); + Assert.Equal(["Install", "Upgrade"], sections.Select(section => section.Title)); + } + + [Fact] + public void Build_HangsASectionOffTheDivisionContainingIt() + { + var objects = BuildGuide(); + + var article = Assert.Single(objects, entry => entry.ObjectType == KnowledgeObjectTypes.Article); + + foreach (var section in objects.Where(entry => entry.ObjectType == KnowledgeObjectTypes.Section)) + { + // A section belongs to its chapter, not to the document. Hanging it off the root would make the + // nesting the analyzer worked out invisible to anything reading the store back. + Assert.Equal(article.CanonicalId, section.ParentId); + } + } + + [Fact] + public void Build_KeepsASectionsOwnTextOutOfItsParent() + { + var objects = BuildGuide(); + + var article = Assert.Single(objects, entry => entry.ObjectType == KnowledgeObjectTypes.Article); + var install = objects.Single(entry => entry.Title == "Install"); + + Assert.Contains("Steps.", install.Content, StringComparison.Ordinal); + + // The chapter keeps its own introduction and nothing belonging to the sections beneath it, or the + // document's text would be stored once per level of nesting. + Assert.Contains("Intro.", article.Content, StringComparison.Ordinal); + Assert.DoesNotContain("Steps.", article.Content, StringComparison.Ordinal); + } + + [Fact] + public void Build_StoresAFlatDocumentAsArticlesOnly() + { + // Nothing nested means nothing stored as a section, so a document that was never hierarchical is + // stored exactly as it was before sections existed. + var document = new IngestionDocument("notes.txt"); + var section = new IngestionDocumentSection(); + + section.Elements.Add(new IngestionDocumentParagraph("Just a paragraph.") + { + Text = "Just a paragraph.", + }); + + document.Sections.Add(section); + + var objects = Build(document); + + Assert.DoesNotContain(objects, entry => entry.ObjectType == KnowledgeObjectTypes.Section); + Assert.Contains(objects, entry => entry.ObjectType == KnowledgeObjectTypes.Article); + } + + private static IReadOnlyList BuildGuide() + { + var document = HtmlIngestionDocumentReader.Read( + "

Guide

Intro.

Install

Steps.

Upgrade

More.

", + "https://example.test/guide"); + + return Build(document); + } + + private static IReadOnlyList Build(IngestionDocument document) + { + var structure = StructureAnalyzers.Default().Analyze(document); + + return KnowledgeObjectBuilder.Build( + document, + new KnowledgeObjectBuildOptions + { + FileKey = "0123456789abcdef", + DataSourceId = "data-source-1", + Title = "guide.html", + ContentHash = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + Structure = structure, + }, + []); + } +} diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/OutlineStructureTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/OutlineStructureTests.cs index d7159678..6d0e8fa8 100644 --- a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/OutlineStructureTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/OutlineStructureTests.cs @@ -1,5 +1,6 @@ using CrestApps.Core.AI.Ingestion.Knowledge.Structure; using CrestApps.Core.AI.Ingestion.Pdf.Services; +using CrestApps.Core.Tests.Support; using Microsoft.Extensions.Logging.Abstractions; using UglyToad.PdfPig.Content; using UglyToad.PdfPig.Fonts.Standard14Fonts; @@ -106,7 +107,7 @@ public async Task Analyze_GivesAPageToItsInnermostDivision() new OutlineFixture("Section 2.1", 2, 5))); var document = await ReadAsync(pdf); - new TocSeededStructureAnalyzer(NullLogger.Instance).Analyze(document); + StructureAnalyzers.Default().Analyze(document); var structure = await AnalyzeAsync(pdf); var byTitle = structure.Articles.ToDictionary(article => article.Title, StringComparer.Ordinal); @@ -151,7 +152,7 @@ private static async Task AnalyzeAsync(byte[] pdf) { var document = await ReadAsync(pdf); - return new TocSeededStructureAnalyzer(NullLogger.Instance).Analyze(document); + return StructureAnalyzers.Default().Analyze(document); } private static async Task ReadAsync(byte[] pdf) diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StatedHeadingStructureTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StatedHeadingStructureTests.cs index b8492b81..52c7d1db 100644 --- a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StatedHeadingStructureTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StatedHeadingStructureTests.cs @@ -2,6 +2,7 @@ using CrestApps.Core.AI.Ingestion; using CrestApps.Core.AI.Ingestion.Knowledge.Structure; using CrestApps.Core.DataIngestion; +using CrestApps.Core.Tests.Support; using DocumentFormat.OpenXml; using DocumentFormat.OpenXml.Packaging; using DocumentFormat.OpenXml.Wordprocessing; @@ -137,7 +138,7 @@ public void Html_WithASingleHeadingStaysOneDivision() private static DocumentStructure Analyze(IngestionDocument document) { - return new TocSeededStructureAnalyzer(NullLogger.Instance).Analyze(document); + return StructureAnalyzers.Default().Analyze(document); } private static int GetHeadingLevel(IngestionDocumentElement element) diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StructureLadderTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StructureLadderTests.cs index 9e4f1661..ebaa6f34 100644 --- a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StructureLadderTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StructureLadderTests.cs @@ -1,6 +1,7 @@ using CrestApps.Core.AI.Ingestion; using CrestApps.Core.AI.Ingestion.Knowledge.Structure; using CrestApps.Core.DataIngestion; +using CrestApps.Core.Tests.Support; using Microsoft.Extensions.DataIngestion; using Microsoft.Extensions.Logging.Abstractions; @@ -106,6 +107,6 @@ private static IngestionDocumentSection BuildSection(int page, params (string Te private static DocumentStructure Analyze(IngestionDocument document) { - return new TocSeededStructureAnalyzer(NullLogger.Instance).Analyze(document); + return StructureAnalyzers.Default().Analyze(document); } } diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Knowledge/TocSeededStructureAnalyzerTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/TableOfContentsStructureTests.cs similarity index 99% rename from tests/CrestApps.Core.Tests/Core/Ingestion/Knowledge/TocSeededStructureAnalyzerTests.cs rename to tests/CrestApps.Core.Tests/Core/Ingestion/Structure/TableOfContentsStructureTests.cs index a15521de..31188703 100644 --- a/tests/CrestApps.Core.Tests/Core/Ingestion/Knowledge/TocSeededStructureAnalyzerTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/TableOfContentsStructureTests.cs @@ -1,3 +1,4 @@ +using CrestApps.Core.Tests.Support; using CrestApps.Core.AI.Ingestion; using CrestApps.Core.AI.Ingestion.Knowledge; using CrestApps.Core.AI.Ingestion.Knowledge.Structure; @@ -6,14 +7,14 @@ using Microsoft.Extensions.DataIngestion; using Microsoft.Extensions.Logging.Abstractions; -namespace CrestApps.Core.Tests.Core.Ingestion.Knowledge; +namespace CrestApps.Core.Tests.Core.Ingestion.Structure; /// /// Covers splitting a document into the articles it is actually made of. A magazine indexed as one document /// answers every question with a blend of thirty unrelated subjects; split correctly, each article answers /// for itself. /// -public sealed class TocSeededStructureAnalyzerTests +public sealed class TableOfContentsStructureTests { private const double BodySize = 10; private const double HeadingSize = 20; @@ -533,7 +534,7 @@ private static IngestionDocumentParagraph Plain(string text) private static DocumentStructure Analyze(IngestionDocument document) { - return new TocSeededStructureAnalyzer(NullLogger.Instance).Analyze(document); + return StructureAnalyzers.Default().Analyze(document); } /// diff --git a/tests/CrestApps.Core.Tests/Support/FileSourceTestPipeline.cs b/tests/CrestApps.Core.Tests/Support/FileSourceTestPipeline.cs index 191df562..32ae44be 100644 --- a/tests/CrestApps.Core.Tests/Support/FileSourceTestPipeline.cs +++ b/tests/CrestApps.Core.Tests/Support/FileSourceTestPipeline.cs @@ -40,7 +40,7 @@ public static DefaultKnowledgeIngestionService Create( store, fileStore ?? new RecordingDocumentFileStore(), new DefaultAITextNormalizer(), - new TocSeededStructureAnalyzer(NullLogger.Instance), + StructureAnalyzers.Default(), new NullPublicationMetadataExtractor(), queue ?? new RecordingIndexingQueue(), NullLogger.Instance); diff --git a/tests/CrestApps.Core.Tests/Support/StructureAnalyzers.cs b/tests/CrestApps.Core.Tests/Support/StructureAnalyzers.cs new file mode 100644 index 00000000..2ff4fbd2 --- /dev/null +++ b/tests/CrestApps.Core.Tests/Support/StructureAnalyzers.cs @@ -0,0 +1,35 @@ +using CrestApps.Core.AI.Ingestion.Knowledge.Structure; +using Microsoft.Extensions.Logging.Abstractions; + +namespace CrestApps.Core.Tests.Support; + +/// +/// Builds the structure analyzer the way the service registration does. +/// +/// +/// The analyzer is only as good as the strategies it is given, so a test that built it with none would pass +/// while proving nothing. This assembles the same ladder a host gets, in the same order, and is the only +/// place a test should construct one. +/// +internal static class StructureAnalyzers +{ + /// + /// Creates an analyzer carrying the built-in ladder. + /// + /// Strategies to add to the built-in ones, for tests that contribute a rung. + /// The analyzer. + public static DocumentStructureAnalyzer Default(params IDocumentStructureStrategy[] extra) + { + IDocumentStructureStrategy[] builtIn = + [ + new OutlineStructureStrategy(), + new StatedHeadingStructureStrategy(), + new TableOfContentsStructureStrategy(), + new InferredHeadingStructureStrategy(), + ]; + + return new DocumentStructureAnalyzer( + [.. builtIn, .. extra], + NullLogger.Instance); + } +} From 05957b5ea10c0a247e4134d8a3efc0a8dfe39971 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Sun, 20 Sep 2026 10:45:41 -0700 Subject: [PATCH 6/8] Read a wrapped headline as one headline, and overprinted type once Two defects real publications have and synthetic fixtures never did. Both come from how type is set rather than from how a document is structured, which is why reasoning about the code did not find them and running magazines through it did. A headline that wraps is one headline. Its second line is another run of type at the same size directly beneath the first, and the rule that lets a newspaper page carry four stories was reading it as a fifth: the article split in two, the first half titled with half a sentence, and the pages handed to the half that said least. Two lines now merge when they share a size, sit within ordinary leading of each other, and overlap across the measure. All three conditions are required -- same size alone would swallow the standfirst beneath a headline, and adjacency alone would swallow the first line of body text set large. Two headlines in adjacent columns share a size and a height and fail the overlap test, which is what keeps them two headlines. Display type is routinely set twice with a slight offset, to fake a weight the font does not have. Both runs are real text, so a reader that takes the page at its word reads every headline word twice. A word repeated immediately after itself is now collapsed, in headings only; body text is left alone, and English headings that legitimately repeat a word next to themselves are vanishingly rare. Measured on a fifteen page research paper, a seventy page magazine and a twenty-three page trade journal. Structure came from the outline where one existed and from type size where none did; no element in any of the three ended up belonging to no division; figures, charts and tables were separated, and captions were matched. The two fixes took the magazines from thirty-nine and twenty-eight divisions to twenty-seven and twenty. What remains is what inference cannot do: a masthead, a strapline and an advertisement look like headlines, so the opening pages of a magazine still yield divisions that are not articles. The contents-page rung handles that properly wherever a contents page can be read. Co-Authored-By: Claude Opus 5 --- .../general-document-structure-engine.md | 38 +++- .../Structure/DocumentStructureRungs.cs | 162 +++++++++++++++++- .../Structure/RealDocumentHeadingTests.cs | 150 ++++++++++++++++ 3 files changed, 342 insertions(+), 8 deletions(-) create mode 100644 tests/CrestApps.Core.Tests/Core/Ingestion/Structure/RealDocumentHeadingTests.cs diff --git a/docs/design/general-document-structure-engine.md b/docs/design/general-document-structure-engine.md index c3fc8502..d3cda6b1 100644 --- a/docs/design/general-document-structure-engine.md +++ b/docs/design/general-document-structure-engine.md @@ -106,12 +106,36 @@ Advertisements left the general path with this. A page carrying no article and n something in a magazine and nothing in a manual, so `SplitAdvertisements` runs only on the table-of-contents rung rather than on every document that reaches `BuildArticles`. +### What real documents taught it + +Running actual publications through the pipeline found two defects that no synthetic fixture would +ever have shown, because both come from how type is set rather than from how a document is +structured. + +**A headline that wraps is one headline.** Its second line is another run of type at the same size +directly beneath the first, and the rule that lets a newspaper page carry four stories was reading +it as a fifth. The article was split in two, the first half titled with half a sentence, and the +pages given to the half that said least. Two lines now merge when they share a size, sit within +ordinary leading of each other, and overlap across the measure — all three, because same size alone +would swallow a standfirst and adjacency alone would swallow the first line of body text. + +**Display type is routinely set twice.** A slight offset fakes a weight the font does not have, and +both runs are real text, so every word of the headline is read twice: +`COMMAND COMMAND & & CONQUER`. A word repeated immediately after itself is now collapsed, in +headings only. + +Measured on a fifteen page research paper, a seventy page magazine and a twenty-three page trade +journal: structure came from the outline where one existed and from type size where none did, no +element in any of the three ended up belonging to no division, and figures, charts and tables were +separated and captioned. The two fixes took the magazines from thirty-nine and twenty-eight +divisions to twenty-seven and twenty. + ### What is still open -**Fixtures are synthetic.** Every test builds the document it reads — a bookmarked manual, a Word -file with heading styles, an HTML page, a page carrying three stories. That proves the algorithms -against documents written the way the specifications say. It proves nothing about a real magazine -whose contents page is set as a picture, a manual whose outline points at the wrong pages, a -newspaper whose headlines are set in the same size as its standfirsts, or a PDF that claims tags it -does not honour. Real documents are where the next class of failure lives, and measuring against -them is what would turn this from correct into trustworthy. +**Covers and advertising pages still produce divisions.** Type-size inference has no way to tell a +masthead, a strapline or an advertisement from a headline, so the opening pages of a magazine yield +divisions that are not articles. The contents-page rung handles this properly where a contents page +can be read; where one cannot, this is the cost of inferring. + +**Overprinted type can still fragment.** Where the two runs land far enough apart to segment +separately, they survive as two blocks, and the second becomes a short division of its own. diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureRungs.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureRungs.cs index 876b1a7c..4e871351 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureRungs.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureRungs.cs @@ -707,6 +707,7 @@ internal static List InferHeadingBoundaries(IngestionDocument document var pageHeight = GetDouble(section, ElementMetadataKeys.PageHeight) ?? 0; var page = GetPageNumber(document, index); var onThisPage = 0; + IngestionDocumentElement previous = null; foreach (var element in section.Elements) { @@ -747,7 +748,25 @@ internal static List InferHeadingBoundaries(IngestionDocument document continue; } + // A headline that wraps is one headline. Its second line is the same size, directly beneath + // the first and overlapping it across the page, and opening a division at it would file the + // article under half a sentence and leave the other half titling nothing. + if (onThisPage > 0 && ContinuesHeading(previous, element, size.Value)) + { + var merged = boundaries[^1]; + + boundaries[^1] = merged with + { + Seed = merged.Seed with { Title = CollapseHeading($"{merged.Seed.Title} {text}") }, + }; + + previous = element; + + continue; + } + onThisPage++; + previous = element; boundaries.Add(new Boundary(page, new TocSeed(text, null, page), position)); } } @@ -760,6 +779,100 @@ internal static List InferHeadingBoundaries(IngestionDocument document /// /// The element. /// The top coordinate, or when the element carries no bounds. + /// + /// How much bigger or smaller a block may be than the line above it and still be the same headline. + /// + internal const double HeadingContinuationSizeTolerance = 0.12; + + /// + /// How far below a headline its next line may begin, as a multiple of the type size. + /// + /// + /// Leading is close to the type size in display setting and rarely more than twice it. A gap larger than + /// this is a different piece of type, not the rest of this one. + /// + internal const double HeadingContinuationLeading = 2.0; + + /// + /// Decides whether a block is the next line of the heading above it rather than a heading of its own. + /// + /// The block accepted as a heading immediately before this one. + /// The block being considered. + /// The block's type size. + /// when the block continues the heading above it. + /// + /// A headline that wraps produces two blocks the segmenter has no reason to join: they are separate runs + /// of type at the same size, one directly beneath the other, overlapping across the measure. Treating + /// the second as a heading of its own splits one article into two, titles the first with half a sentence + /// and gives the article's pages to the half that says least. + /// + /// All three conditions are required. Same size alone would join a headline to the standfirst beneath + /// it; adjacency alone would join a headline to the first line of body text set large. + /// + /// + internal static bool ContinuesHeading(IngestionDocumentElement previous, IngestionDocumentElement element, double size) + { + if (previous is null) + { + return false; + } + + var previousSize = GetDouble(previous, ElementMetadataKeys.ModalPointSize); + + if (previousSize is null || previousSize.Value <= 0) + { + return false; + } + + if (Math.Abs(size - previousSize.Value) > previousSize.Value * HeadingContinuationSizeTolerance) + { + return false; + } + + var above = GetBounds(previous); + var below = GetBounds(element); + + if (above is null || below is null) + { + return false; + } + + // The next line sits below the one above it, by no more than ordinary leading. + var gap = above.Value.Bottom - below.Value.Top; + + if (gap < -previousSize.Value || gap > previousSize.Value * HeadingContinuationLeading) + { + return false; + } + + // And beneath it rather than beside it: two headlines in adjacent columns are the same size and the + // same height, and are two headlines. + var overlap = Math.Min(above.Value.Right, below.Value.Right) - Math.Max(above.Value.Left, below.Value.Left); + var narrowest = Math.Min(above.Value.Right - above.Value.Left, below.Value.Right - below.Value.Left); + + return narrowest > 0 && overlap > narrowest / 2; + } + + /// + /// Reads an element's bounds. + /// + /// The element. + /// The bounds, or when the element carries none. + internal static (double Left, double Bottom, double Right, double Top)? GetBounds(IngestionDocumentElement element) + { + if (!element.HasMetadata || !element.Metadata.TryGetValue(ElementMetadataKeys.BoundingBox, out var value)) + { + return null; + } + + if (value is not double[] { Length: 4 } bounds) + { + return null; + } + + return (bounds[0], bounds[1], bounds[2], bounds[3]); + } + internal static double? GetTop(IngestionDocumentElement element) { if (!element.HasMetadata || @@ -784,13 +897,60 @@ internal static string CollapseHeading(string text) return string.Empty; } - var collapsed = WhitespaceRun().Replace(text, " ").Trim(); + var collapsed = CollapseOverprint(WhitespaceRun().Replace(text, " ").Trim()); return collapsed.Length <= MaxInferredTitleCharacters ? collapsed : collapsed[..MaxInferredTitleCharacters].TrimEnd(); } + /// + /// Collapses a word repeated immediately after itself. + /// + /// The heading text. + /// The heading with overprinting removed. + /// + /// Display type is routinely set twice, slightly offset, to fake a weight the font does not have. Both + /// runs are real text, so a reader that takes a page at its word reads every headline word twice: + /// "COMMAND COMMAND & & CONQUER". Nothing downstream can tell that from a title, and it is the + /// title a reader of the knowledge base sees. + /// + /// Only an immediate repeat is collapsed, and only in a heading. English headings that legitimately + /// repeat a word next to itself are vanishingly rare, and body text is left alone entirely. + /// + /// + internal static string CollapseOverprint(string text) + { + if (text.Length == 0 || !text.Contains(' ', StringComparison.Ordinal)) + { + return text; + } + + var words = text.Split(' ', StringSplitOptions.RemoveEmptyEntries); + + if (words.Length < 2) + { + return text; + } + + var kept = new List(words.Length) { words[0] }; + var collapsed = false; + + for (var index = 1; index < words.Length; index++) + { + if (string.Equals(words[index], kept[^1], StringComparison.Ordinal)) + { + collapsed = true; + + continue; + } + + kept.Add(words[index]); + } + + return collapsed ? string.Join(' ', kept) : text; + } + internal static List BuildArticles( IngestionDocument document, List boundaries, diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/RealDocumentHeadingTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/RealDocumentHeadingTests.cs new file mode 100644 index 00000000..d2c6a518 --- /dev/null +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/RealDocumentHeadingTests.cs @@ -0,0 +1,150 @@ +using CrestApps.Core.AI.Ingestion.Knowledge.Structure; +using CrestApps.Core.AI.Ingestion.Pdf.Services; +using CrestApps.Core.Tests.Support; +using UglyToad.PdfPig.Content; +using UglyToad.PdfPig.Core; +using UglyToad.PdfPig.Fonts.Standard14Fonts; +using UglyToad.PdfPig.Writer; + +namespace CrestApps.Core.Tests.Core.Ingestion.Structure; + +/// +/// Covers two things real magazines do that synthetic fixtures never did. +/// +/// +/// Both were found by running actual magazines through the pipeline rather than by reasoning about it. A +/// headline that wraps was being read as two articles, and display type set twice to fake a heavier weight +/// was being read as every word repeated. +/// +public sealed class RealDocumentHeadingTests +{ + private const string PdfMediaType = "application/pdf"; + + private static readonly string[] BodyLines = + [ + "The body of the article begins here and runs on for a while.", + "It continues with a second sentence of ordinary prose.", + "A third line keeps the body the commonest size on the page.", + "And a fourth settles the matter beyond any doubt.", + ]; + + [Fact] + public async Task Analyze_ReadsAWrappedHeadlineAsOneArticle() + { + // The first article's headline runs over two lines. Reading the second line as a headline of its own + // splits the article in two, titles the first half with half a sentence, and gives the article's + // pages to the half that says least. + var pdf = CreateIssue( + new Article(["The consequences of the regulatory changes", "for the energy rating of dwellings"]), + new Article(["Summer comfort in dwellings"]), + new Article(["Electrostatic filtration indoors"])); + + var structure = await AnalyzeAsync(pdf); + + Assert.Equal(3, structure.Articles.Count); + + Assert.Equal( + "The consequences of the regulatory changes for the energy rating of dwellings", + structure.Articles[0].Title); + + // And the merged headline keeps the article's pages, rather than its second line taking them. + Assert.Equal(1, structure.Articles[0].PageStart); + Assert.Equal(1, structure.Articles[0].PageEnd); + Assert.Equal("Summer comfort in dwellings", structure.Articles[1].Title); + } + + [Fact] + public async Task Analyze_KeepsTwoHeadlinesSideBySideApart() + { + // Two headlines in adjacent columns are the same size and the same height. They are two headlines, + // and a rule that merged on size and proximity alone would join them into one. + var pdf = CreateIssue( + new Article(["Council approves budget"], SecondColumnHeadline: "Bridge reopens today"), + new Article(["Library extends its hours"])); + + var structure = await AnalyzeAsync(pdf); + + Assert.Equal(3, structure.Articles.Count); + + Assert.Equal( + ["Council approves budget", "Bridge reopens today", "Library extends its hours"], + structure.Articles.Select(article => article.Title)); + } + + [Theory] + [InlineData("COMMAND COMMAND & & CONQUER", "COMMAND & CONQUER")] + [InlineData("Inkscape Inkscape - - Part Part 158", "Inkscape - Part 158")] + [InlineData("Trading Trading Up Up To To Linux Linux Pt.6", "Trading Up To Linux Pt.6")] + [InlineData("Nothing repeated here", "Nothing repeated here")] + [InlineData("", "")] + [InlineData("Single", "Single")] + public void CollapseOverprint_RemovesTypeSetTwice(string printed, string expected) + { + // Display type set twice with a slight offset to fake a heavier weight reads as every word repeated. + // Both runs are real text, so nothing downstream can tell this from a title. + Assert.Equal(expected, DocumentStructureRungs.CollapseOverprint(printed)); + } + + private static async Task AnalyzeAsync(byte[] pdf) + { + using var stream = new MemoryStream(pdf, writable: false); + var document = await new PdfIngestionDocumentReader().ReadAsync(stream, "issue.pdf", PdfMediaType); + + return StructureAnalyzers.Default().Analyze(document); + } + + /// + /// Builds an issue with one article per page. + /// + /// The articles, in order. + /// The PDF bytes. + private static byte[] CreateIssue(params Article[] articles) + { + var builder = new PdfDocumentBuilder(); + var font = builder.AddStandard14Font(Standard14Font.Helvetica); + + foreach (var article in articles) + { + var page = builder.AddPage(PageSize.A4); + var top = 780d; + + foreach (var line in article.HeadlineLines) + { + page.AddText(line, 22, new PdfPoint(40, top), font); + top -= 28; + } + + // Enough body text that the body size is the commonest size on the page. A page that is mostly + // headline makes the headline the body, and then nothing on it reads as a heading at all. + var body = top - 24; + + foreach (var line in BodyLines) + { + page.AddText(line, 10, new PdfPoint(40, body), font); + body -= 14; + } + + if (article.SecondColumnHeadline is not null) + { + page.AddText(article.SecondColumnHeadline, 22, new PdfPoint(320, 780), font); + + var beside = 742d; + + foreach (var line in BodyLines) + { + page.AddText(line, 10, new PdfPoint(320, beside), font); + beside -= 14; + } + } + } + + return builder.Build(); + } + + /// + /// One article to place on its own page. + /// + /// The headline, one entry per printed line. + /// A second story beside it, when the page carries one. + private sealed record Article(string[] HeadlineLines, string SecondColumnHeadline = null); +} From cc0c8359b435b07988149f8e4d7b6fca81b2e82d Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Mon, 21 Sep 2026 07:19:31 -0700 Subject: [PATCH 7/8] Put the ingestion metadata keys below the readers that write them Earlier in this branch the HTML reader took a reference on the AI ingestion package so it could spell one constant, ElementMetadataKeys.HeadingLevel. That was described at the time as one more assembly. It was not. CrestApps.Core.DataIngestion turns HTML into text. Through that one reference it was carrying eight CrestApps assemblies, Lucene, ZString, Microsoft.Extensions.AI and a framework reference to ASP.NET Core. Its dependency closure is now itself and CrestApps.Core.Abstractions, and nothing else. ElementMetadataKeys moves to CrestApps.Core.Abstractions, under CrestApps.Core.Ingestion. It is a bag of const strings and it belongs there on principle rather than convenience: every reader writes these keys, whatever it reads and whatever it is read for, so they are a contract, and a contract belongs below the things that implement it. A reader should be able to state that a line is a heading without taking on the machinery that later decides what to do about it. The namespace is CrestApps.Core.Ingestion rather than the AI one it came from, so the assembly and the namespace agree. The keys were added during 2.0.0 development and have never shipped, so this is where they live rather than a move anyone has to follow. Nothing else changed: no registration, no behaviour, and the three readers that legitimately need the ingestion package -- PDF, OpenXml and Document Intelligence all use MediaTypeHelper, the reader registration helper or the outline type -- keep it. A prune pass over every file holding a `using` for that namespace removed none, which is the evidence that no other file was coupled to it for the keys alone. Co-Authored-By: Claude Opus 5 --- .../design/general-document-structure-engine.md | 15 +++++++++++++++ .../Ingestion}/ElementMetadataKeys.cs | 17 +++++++++++------ .../Services/OpenXmlIngestionDocumentReader.cs | 1 + .../DefaultAIDocumentProcessingService.cs | 1 + .../Services/PdfIngestionDocumentReader.cs | 1 + .../IngestionDocumentElementExtensions.cs | 1 + .../DefaultKnowledgeIngestionService.cs | 1 + .../Knowledge/KnowledgeObjectBuilder.cs | 1 + .../Structure/DocumentStructureRungs.cs | 1 + .../DefaultFigureCaptionCandidateDetector.cs | 1 + .../Processors/FigureCaptionProcessor.cs | 1 + .../Processors/FigureGeometry.cs | 1 + .../Processors/FigureSalienceProcessor.cs | 1 + .../DocumentIntelligenceDocumentMapper.cs | 1 + .../CrestApps.Core.DataIngestion.csproj | 2 +- .../HtmlIngestionDocumentReader.cs | 2 +- .../DocumentIntelligenceDocumentMapperTests.cs | 1 + .../FallbackIngestionDocumentReaderTests.cs | 1 + .../Knowledge/KnowledgeObjectBuilderTests.cs | 1 + .../Pdf/PdfIngestionDocumentReaderTests.cs | 1 + .../Processors/FigureCaptionProcessorTests.cs | 1 + .../Processors/FigureSalienceProcessorTests.cs | 1 + .../Ingestion/Structure/MultiStoryPageTests.cs | 3 ++- .../Structure/OutlineStructureTests.cs | 3 ++- .../Structure/StatedHeadingStructureTests.cs | 1 + .../Ingestion/Structure/StructureLadderTests.cs | 1 + .../Structure/TableOfContentsStructureTests.cs | 1 + 27 files changed, 53 insertions(+), 10 deletions(-) rename src/{Primitives/CrestApps.Core.AI.Ingestion => Abstractions/CrestApps.Core.Abstractions/Ingestion}/ElementMetadataKeys.cs (83%) diff --git a/docs/design/general-document-structure-engine.md b/docs/design/general-document-structure-engine.md index d3cda6b1..8e57af3e 100644 --- a/docs/design/general-document-structure-engine.md +++ b/docs/design/general-document-structure-engine.md @@ -69,6 +69,21 @@ Rung 2 collapsed what were going to be three separate rungs. A Word paragraph st `ElementMetadataKeys.HeadingLevel` carries it from every reader and one rung divides on it. Adding a format means teaching its reader to write that key, not writing another strategy. +### Where the keys live, and why it is not here + +`ElementMetadataKeys` sits in `CrestApps.Core.Abstractions`, not in the ingestion package, because +every reader writes them and not every reader is about AI. `CrestApps.Core.DataIngestion` turns HTML +into text; it needs the key for a heading level and nothing else in this document. + +Holding the keys in the ingestion package meant that reader referencing it for one `const string`, +and taking with it eight CrestApps assemblies, Lucene, ZString and a framework reference to +ASP.NET Core — to spell a constant. Its dependency closure is now itself and one abstractions +assembly. + +The general rule this follows: what every reader must agree on is a contract, and a contract belongs +below the things that implement it. A reader should be able to state that a line is a heading without +taking on the machinery that later decides what to do about it. + ### What each reader now states | Reader | Writes | diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/ElementMetadataKeys.cs b/src/Abstractions/CrestApps.Core.Abstractions/Ingestion/ElementMetadataKeys.cs similarity index 83% rename from src/Primitives/CrestApps.Core.AI.Ingestion/ElementMetadataKeys.cs rename to src/Abstractions/CrestApps.Core.Abstractions/Ingestion/ElementMetadataKeys.cs index 76df59ee..fbc25315 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/ElementMetadataKeys.cs +++ b/src/Abstractions/CrestApps.Core.Abstractions/Ingestion/ElementMetadataKeys.cs @@ -1,9 +1,15 @@ -namespace CrestApps.Core.AI.Ingestion; +namespace CrestApps.Core.Ingestion; /// -/// The metadata keys readers and processors use on any . -/// Every key is written and read through these constants so no string literal can drift. +/// The metadata keys readers and processors use on an ingestion document's sections and elements. Every key +/// is written and read through these constants so no string literal can drift. /// +/// +/// These live at the bottom of the stack because every reader writes them, whatever it reads and whatever it +/// is read for. A reader that turns HTML into text needs the key for a heading level and nothing else about +/// AI ingestion; holding the keys any higher would mean that reader taking the whole AI stack to spell one +/// constant. +/// public static class ElementMetadataKeys { /// @@ -65,9 +71,8 @@ public static class ElementMetadataKeys public const string PageHeight = "crestapps.pageHeight"; /// - /// The document's own outline, as an - /// of in document - /// order. + /// The document's own outline, as an of + /// CrestApps.Core.AI.Ingestion.Knowledge.Structure.DocumentOutlineEntry in document order. /// /// /// Recorded on the first section rather than per page, because it describes the whole file and diff --git a/src/Primitives/CrestApps.Core.AI.Documents.OpenXml/Services/OpenXmlIngestionDocumentReader.cs b/src/Primitives/CrestApps.Core.AI.Documents.OpenXml/Services/OpenXmlIngestionDocumentReader.cs index 2422f6d1..1fae2723 100644 --- a/src/Primitives/CrestApps.Core.AI.Documents.OpenXml/Services/OpenXmlIngestionDocumentReader.cs +++ b/src/Primitives/CrestApps.Core.AI.Documents.OpenXml/Services/OpenXmlIngestionDocumentReader.cs @@ -1,5 +1,6 @@ using System.Text; using CrestApps.Core.AI.Ingestion; +using CrestApps.Core.Ingestion; using DocumentFormat.OpenXml.Packaging; using DocumentFormat.OpenXml.Spreadsheet; using Microsoft.Extensions.DataIngestion; diff --git a/src/Primitives/CrestApps.Core.AI.Documents/Services/DefaultAIDocumentProcessingService.cs b/src/Primitives/CrestApps.Core.AI.Documents/Services/DefaultAIDocumentProcessingService.cs index 2ee9eae9..c84116d3 100644 --- a/src/Primitives/CrestApps.Core.AI.Documents/Services/DefaultAIDocumentProcessingService.cs +++ b/src/Primitives/CrestApps.Core.AI.Documents/Services/DefaultAIDocumentProcessingService.cs @@ -4,6 +4,7 @@ using CrestApps.Core.AI.Ingestion; using CrestApps.Core.AI.Models; using CrestApps.Core.AI.Services; +using CrestApps.Core.Ingestion; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.AI; using Microsoft.Extensions.DataIngestion; diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion.Pdf/Services/PdfIngestionDocumentReader.cs b/src/Primitives/CrestApps.Core.AI.Ingestion.Pdf/Services/PdfIngestionDocumentReader.cs index 4e7d9912..b1a4d03d 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion.Pdf/Services/PdfIngestionDocumentReader.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion.Pdf/Services/PdfIngestionDocumentReader.cs @@ -1,6 +1,7 @@ using System.Security.Cryptography; using System.Text.Json; using CrestApps.Core.AI.Ingestion.Knowledge.Structure; +using CrestApps.Core.Ingestion; using Microsoft.Extensions.DataIngestion; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/IngestionDocumentElementExtensions.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/IngestionDocumentElementExtensions.cs index 5cd06faa..252db0eb 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/IngestionDocumentElementExtensions.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/IngestionDocumentElementExtensions.cs @@ -1,4 +1,5 @@ using System.Text; +using CrestApps.Core.Ingestion; using Microsoft.Extensions.DataIngestion; namespace CrestApps.Core.AI.Ingestion; diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/DefaultKnowledgeIngestionService.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/DefaultKnowledgeIngestionService.cs index 7968e215..161dea91 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/DefaultKnowledgeIngestionService.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/DefaultKnowledgeIngestionService.cs @@ -5,6 +5,7 @@ using CrestApps.Core.AI.Models; using CrestApps.Core.AI.Services; using CrestApps.Core.Infrastructure.Indexing; +using CrestApps.Core.Ingestion; using Microsoft.Extensions.DataIngestion; using Microsoft.Extensions.Logging; diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/KnowledgeObjectBuilder.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/KnowledgeObjectBuilder.cs index e7d26639..555d367e 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/KnowledgeObjectBuilder.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/KnowledgeObjectBuilder.cs @@ -4,6 +4,7 @@ using CrestApps.Core.AI.Ingestion.Processors; using CrestApps.Core.AI.Models; using CrestApps.Core.Infrastructure.Indexing; +using CrestApps.Core.Ingestion; using Microsoft.Extensions.DataIngestion; namespace CrestApps.Core.AI.Ingestion.Knowledge; diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureRungs.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureRungs.cs index 4e871351..55aa8818 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureRungs.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Knowledge/Structure/DocumentStructureRungs.cs @@ -1,4 +1,5 @@ using System.Text.RegularExpressions; +using CrestApps.Core.Ingestion; using Microsoft.Extensions.DataIngestion; using Microsoft.Extensions.Logging; diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Processors/DefaultFigureCaptionCandidateDetector.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Processors/DefaultFigureCaptionCandidateDetector.cs index 0f539523..a330ea00 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/Processors/DefaultFigureCaptionCandidateDetector.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Processors/DefaultFigureCaptionCandidateDetector.cs @@ -1,5 +1,6 @@ using System.Globalization; using System.Text.RegularExpressions; +using CrestApps.Core.Ingestion; using Microsoft.Extensions.Options; namespace CrestApps.Core.AI.Ingestion.Processors; diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Processors/FigureCaptionProcessor.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Processors/FigureCaptionProcessor.cs index 8fb55f47..a93cb0a9 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/Processors/FigureCaptionProcessor.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Processors/FigureCaptionProcessor.cs @@ -1,4 +1,5 @@ using System.Text; +using CrestApps.Core.Ingestion; using Microsoft.Extensions.DataIngestion; using Microsoft.Extensions.Options; diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Processors/FigureGeometry.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Processors/FigureGeometry.cs index c3412853..e3b6fec4 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/Processors/FigureGeometry.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Processors/FigureGeometry.cs @@ -1,3 +1,4 @@ +using CrestApps.Core.Ingestion; using Microsoft.Extensions.DataIngestion; namespace CrestApps.Core.AI.Ingestion.Processors; diff --git a/src/Primitives/CrestApps.Core.AI.Ingestion/Processors/FigureSalienceProcessor.cs b/src/Primitives/CrestApps.Core.AI.Ingestion/Processors/FigureSalienceProcessor.cs index 0f58130a..1cb46f65 100644 --- a/src/Primitives/CrestApps.Core.AI.Ingestion/Processors/FigureSalienceProcessor.cs +++ b/src/Primitives/CrestApps.Core.AI.Ingestion/Processors/FigureSalienceProcessor.cs @@ -1,3 +1,4 @@ +using CrestApps.Core.Ingestion; using Microsoft.Extensions.DataIngestion; using Microsoft.Extensions.Options; diff --git a/src/Primitives/CrestApps.Core.Azure.DocumentIntelligence/Services/DocumentIntelligenceDocumentMapper.cs b/src/Primitives/CrestApps.Core.Azure.DocumentIntelligence/Services/DocumentIntelligenceDocumentMapper.cs index 164e7e89..dbb63ca3 100644 --- a/src/Primitives/CrestApps.Core.Azure.DocumentIntelligence/Services/DocumentIntelligenceDocumentMapper.cs +++ b/src/Primitives/CrestApps.Core.Azure.DocumentIntelligence/Services/DocumentIntelligenceDocumentMapper.cs @@ -3,6 +3,7 @@ using Azure.AI.DocumentIntelligence; using CrestApps.Core.AI.Ingestion; using CrestApps.Core.AI.Ingestion.Processors; +using CrestApps.Core.Ingestion; using Microsoft.Extensions.DataIngestion; namespace CrestApps.Core.Azure.DocumentIntelligence.Services; diff --git a/src/Primitives/CrestApps.Core.DataIngestion/CrestApps.Core.DataIngestion.csproj b/src/Primitives/CrestApps.Core.DataIngestion/CrestApps.Core.DataIngestion.csproj index b4ee8b06..cb06357f 100644 --- a/src/Primitives/CrestApps.Core.DataIngestion/CrestApps.Core.DataIngestion.csproj +++ b/src/Primitives/CrestApps.Core.DataIngestion/CrestApps.Core.DataIngestion.csproj @@ -18,7 +18,7 @@ - + diff --git a/src/Primitives/CrestApps.Core.DataIngestion/HtmlIngestionDocumentReader.cs b/src/Primitives/CrestApps.Core.DataIngestion/HtmlIngestionDocumentReader.cs index 30f538c2..85009121 100644 --- a/src/Primitives/CrestApps.Core.DataIngestion/HtmlIngestionDocumentReader.cs +++ b/src/Primitives/CrestApps.Core.DataIngestion/HtmlIngestionDocumentReader.cs @@ -2,7 +2,7 @@ using System.Text.RegularExpressions; using AngleSharp.Dom; using AngleSharp.Html.Parser; -using CrestApps.Core.AI.Ingestion; +using CrestApps.Core.Ingestion; using Microsoft.Extensions.DataIngestion; namespace CrestApps.Core.DataIngestion; diff --git a/tests/CrestApps.Core.Tests/Core/Azure/DocumentIntelligence/DocumentIntelligenceDocumentMapperTests.cs b/tests/CrestApps.Core.Tests/Core/Azure/DocumentIntelligence/DocumentIntelligenceDocumentMapperTests.cs index 7a39d585..b7727248 100644 --- a/tests/CrestApps.Core.Tests/Core/Azure/DocumentIntelligence/DocumentIntelligenceDocumentMapperTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Azure/DocumentIntelligence/DocumentIntelligenceDocumentMapperTests.cs @@ -2,6 +2,7 @@ using CrestApps.Core.AI.Ingestion; using CrestApps.Core.AI.Ingestion.Processors; using CrestApps.Core.Azure.DocumentIntelligence.Services; +using CrestApps.Core.Ingestion; using Microsoft.Extensions.DataIngestion; namespace CrestApps.Core.Tests.Core.Azure.DocumentIntelligence; diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/FallbackIngestionDocumentReaderTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/FallbackIngestionDocumentReaderTests.cs index de9a009c..68b91da0 100644 --- a/tests/CrestApps.Core.Tests/Core/Ingestion/FallbackIngestionDocumentReaderTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/FallbackIngestionDocumentReaderTests.cs @@ -1,6 +1,7 @@ using System.Text; using CrestApps.Core.AI.Ingestion; using CrestApps.Core.AI.Ingestion.Processors; +using CrestApps.Core.Ingestion; using Microsoft.Extensions.DataIngestion; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Knowledge/KnowledgeObjectBuilderTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Knowledge/KnowledgeObjectBuilderTests.cs index 0a887a4a..c4cb9010 100644 --- a/tests/CrestApps.Core.Tests/Core/Ingestion/Knowledge/KnowledgeObjectBuilderTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Knowledge/KnowledgeObjectBuilderTests.cs @@ -4,6 +4,7 @@ using CrestApps.Core.AI.Ingestion.Processors; using CrestApps.Core.AI.Models; using CrestApps.Core.Infrastructure.Indexing; +using CrestApps.Core.Ingestion; using Microsoft.Extensions.DataIngestion; namespace CrestApps.Core.Tests.Core.Ingestion.Knowledge; diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Pdf/PdfIngestionDocumentReaderTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Pdf/PdfIngestionDocumentReaderTests.cs index 4e6e7404..7ab9c1fe 100644 --- a/tests/CrestApps.Core.Tests/Core/Ingestion/Pdf/PdfIngestionDocumentReaderTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Pdf/PdfIngestionDocumentReaderTests.cs @@ -1,6 +1,7 @@ using CrestApps.Core.AI.Ingestion; using CrestApps.Core.AI.Ingestion.Pdf; using CrestApps.Core.AI.Ingestion.Pdf.Services; +using CrestApps.Core.Ingestion; using CrestApps.Core.Tests.Support; using Microsoft.Extensions.DataIngestion; using Microsoft.Extensions.Options; diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Processors/FigureCaptionProcessorTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Processors/FigureCaptionProcessorTests.cs index 9ad28ad8..34349be7 100644 --- a/tests/CrestApps.Core.Tests/Core/Ingestion/Processors/FigureCaptionProcessorTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Processors/FigureCaptionProcessorTests.cs @@ -1,6 +1,7 @@ using System.Text.RegularExpressions; using CrestApps.Core.AI.Ingestion; using CrestApps.Core.AI.Ingestion.Processors; +using CrestApps.Core.Ingestion; using Microsoft.Extensions.DataIngestion; using Microsoft.Extensions.Options; diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Processors/FigureSalienceProcessorTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Processors/FigureSalienceProcessorTests.cs index 0b08b615..37493212 100644 --- a/tests/CrestApps.Core.Tests/Core/Ingestion/Processors/FigureSalienceProcessorTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Processors/FigureSalienceProcessorTests.cs @@ -1,5 +1,6 @@ using CrestApps.Core.AI.Ingestion; using CrestApps.Core.AI.Ingestion.Processors; +using CrestApps.Core.Ingestion; using Microsoft.Extensions.DataIngestion; using Microsoft.Extensions.Options; diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/MultiStoryPageTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/MultiStoryPageTests.cs index e4a8ef21..2b47ceb5 100644 --- a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/MultiStoryPageTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/MultiStoryPageTests.cs @@ -1,5 +1,6 @@ using CrestApps.Core.AI.Ingestion.Knowledge.Structure; using CrestApps.Core.AI.Ingestion.Pdf.Services; +using CrestApps.Core.Ingestion; using CrestApps.Core.Tests.Support; using Microsoft.Extensions.Logging.Abstractions; using UglyToad.PdfPig.Content; @@ -55,7 +56,7 @@ public async Task Analyze_KeepsEachStorysOwnTextApart() // headline came last. var ordinals = document.Sections .SelectMany(section => section.Elements) - .Select(element => (int)element.Metadata[CrestApps.Core.AI.Ingestion.ElementMetadataKeys.ArticleOrdinal]) + .Select(element => (int)element.Metadata[ElementMetadataKeys.ArticleOrdinal]) .Distinct() .ToList(); diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/OutlineStructureTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/OutlineStructureTests.cs index 6d0e8fa8..b222a5d5 100644 --- a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/OutlineStructureTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/OutlineStructureTests.cs @@ -1,5 +1,6 @@ using CrestApps.Core.AI.Ingestion.Knowledge.Structure; using CrestApps.Core.AI.Ingestion.Pdf.Services; +using CrestApps.Core.Ingestion; using CrestApps.Core.Tests.Support; using Microsoft.Extensions.Logging.Abstractions; using UglyToad.PdfPig.Content; @@ -115,7 +116,7 @@ public async Task Analyze_GivesAPageToItsInnermostDivision() // Page 2 belongs to both Chapter 1 and Section 1.1. Storing its text against both would store it // twice, so the innermost division owns it. var page2 = document.Sections.Single(section => section.PageNumber == 2); - var ordinal = Assert.IsType(page2.Metadata[CrestApps.Core.AI.Ingestion.ElementMetadataKeys.ArticleOrdinal]); + var ordinal = Assert.IsType(page2.Metadata[ElementMetadataKeys.ArticleOrdinal]); Assert.Equal(byTitle["Section 1.1"].Ordinal, ordinal); } diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StatedHeadingStructureTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StatedHeadingStructureTests.cs index 52c7d1db..89abe08a 100644 --- a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StatedHeadingStructureTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StatedHeadingStructureTests.cs @@ -2,6 +2,7 @@ using CrestApps.Core.AI.Ingestion; using CrestApps.Core.AI.Ingestion.Knowledge.Structure; using CrestApps.Core.DataIngestion; +using CrestApps.Core.Ingestion; using CrestApps.Core.Tests.Support; using DocumentFormat.OpenXml; using DocumentFormat.OpenXml.Packaging; diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StructureLadderTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StructureLadderTests.cs index ebaa6f34..73a121fa 100644 --- a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StructureLadderTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/StructureLadderTests.cs @@ -1,6 +1,7 @@ using CrestApps.Core.AI.Ingestion; using CrestApps.Core.AI.Ingestion.Knowledge.Structure; using CrestApps.Core.DataIngestion; +using CrestApps.Core.Ingestion; using CrestApps.Core.Tests.Support; using Microsoft.Extensions.DataIngestion; using Microsoft.Extensions.Logging.Abstractions; diff --git a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/TableOfContentsStructureTests.cs b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/TableOfContentsStructureTests.cs index 31188703..bab11190 100644 --- a/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/TableOfContentsStructureTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Ingestion/Structure/TableOfContentsStructureTests.cs @@ -1,3 +1,4 @@ +using CrestApps.Core.Ingestion; using CrestApps.Core.Tests.Support; using CrestApps.Core.AI.Ingestion; using CrestApps.Core.AI.Ingestion.Knowledge; From aff91dc08f77308dbb948a2f2f75eecc6fad260f Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Mon, 21 Sep 2026 07:25:39 -0700 Subject: [PATCH 8/8] update claude --- CLAUDE.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..9f1b84ee --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,5 @@ +## Git Commit Rules + +- Never add Claude as a commit author or co-author. +- Never add `Co-authored-by: Claude` or any Claude-related co-author trailer to commits. +- Commits must use only the configured human Git author. \ No newline at end of file