Keep a PDF's title once and never print a chart marker as text

src/CrestApps.Core.Docs/docs/changelog/2.0.0.md

@@ -1741,6 +1741,18 @@ See [Stacked Image Carousel](../core/chat.md#stacked-image-carousel).
 
 ## Change Logs
 
+- **Fixed: a PDF the agent composed showed its title twice and a chart marker as text.** Asked for a report
+  with the charts of the conversation, gpt-4.1 sent the title heading with `create_pdf` and again at the start
+  of `add_pdf_content`, and wrote `[chart:Refer to chart marker above]` as a paragraph under each chart's
+  heading. The file opened with the title twice and printed that line where each chart should be.
+  - a heading that repeats the heading right before it, with nothing between, is left out — within a call and
+    against the block the new content follows — and the agent is told to send only what is new. A heading
+    used again further on is kept
+  - a paragraph that is a whole `[chart:{…}]` marker is drawn as that chart; any other chart marker in a
+    paragraph, heading or list fails the call with the ways to add a chart, and the charts of the
+    conversation it can use. The PDF agent's instructions say never to write a marker in text
+  - content converted from a file is not changed by either rule
+
 - **Fixed: a preview showed a file the reader could not download.** `preview_pdf`, `preview_word`,
   `preview_presentation` and `preview_tabular_data` registered only their pictures; a download existed only
   when the model also called the export tool, and the PDF agent's instructions told it to skip the export when

src/CrestApps.Core.Docs/docs/core/ai-documents.md

@@ -228,6 +228,11 @@ Without it, `sign_pdf` explains that signing is not configured; `verify_pdf_sign
   page shows no number but is counted, so the page after it reads "Page 2 of 3".
 - Positions on a page are points from its top-left, as `search_pdf` reports them; on a page with a
   `/Rotate` entry they are in the page's unrotated frame.
+- A chart goes in as a chart block. A paragraph that is a whole `[chart:{…}]` marker is drawn as that chart;
+  any other chart marker in text — including a placeholder such as `[chart:Refer to chart marker above]` —
+  fails the call with the ways to add the chart, because text is printed as written.
+- A heading that repeats the heading right before it, with nothing between, is left out, and the agent is
+  told. This covers a model that sends the title with `create_pdf` and again at the start of the content.
 - For the Orchard Core integration, see [CrestApps for Orchard Core](https://orchardcore.crestapps.com).
 
 ### Images

src/Primitives/CrestApps.Core.AI.Documents.Pdf/Templates/Prompts/pdf-agent.md

@@ -45,6 +45,8 @@ already listed, and never ask them to upload a file you produced — the working
      not in a document. Put the chart's fields inside the block's `chart` object:
      `{"type": "chart", "chart": {"conversation_chart": 1}}`. A chart with no data fails the call: read the
      error and fix it.
+     Never write a `[chart:...]` marker, or a placeholder such as "[chart: see above]", in a paragraph,
+     heading or list: text is printed as written, so the call fails. Add each chart as a chart block.
      `bar` and `column` draw
      upright bars; use `horizontal_bar` only for long category names. Use the categories and values the
      user gave — "a bar chart of the same numbers" plots exactly the table's rows. Pass `number_format`

src/Primitives/CrestApps.Core.AI.Documents.Pdf/Tools/AddPdfContentTool.cs

@@ -102,7 +102,7 @@ protected override async Task<string> ExecuteAsync(PdfToolArguments arguments, P
                 case "append":
                     {
                         var section = ResolveSection(definition, arguments);
-                        warnings = await PdfBlockPreparer.PrepareAsync(definition, blocks, arguments.Services, cancellationToken);
+                        warnings = await PdfBlockPreparer.PrepareAsync(definition, blocks, arguments.Services, cancellationToken, precedingBlock: LastBlockBefore(definition, section, section.Blocks.Count));
                         section.Blocks.AddRange(blocks);
                         summary = $"Added {Describe(blocks)} to section {section.Id}.";
 
@@ -113,8 +113,9 @@ protected override async Task<string> ExecuteAsync(PdfToolArguments arguments, P
                 case "insert_after":
                     {
                         var (section, index) = FindBlock(definition, target);
-                        warnings = await PdfBlockPreparer.PrepareAsync(definition, blocks, arguments.Services, cancellationToken);
-                        section.Blocks.InsertRange(operation == "insert_before" ? index : index + 1, blocks);
+                        var position = operation == "insert_before" ? index : index + 1;
+                        warnings = await PdfBlockPreparer.PrepareAsync(definition, blocks, arguments.Services, cancellationToken, precedingBlock: LastBlockBefore(definition, section, position));
+                        section.Blocks.InsertRange(position, blocks);
                         summary = $"Inserted {Describe(blocks)} {(operation == "insert_before" ? "before" : "after")} {target}.";
 
                         break;
@@ -124,7 +125,7 @@ protected override async Task<string> ExecuteAsync(PdfToolArguments arguments, P
                     {
                         var (section, index) = FindBlock(definition, target);
                         var replacedId = section.Blocks[index].Id;
-                        warnings = await PdfBlockPreparer.PrepareAsync(definition, blocks, arguments.Services, cancellationToken);
+                        warnings = await PdfBlockPreparer.PrepareAsync(definition, blocks, arguments.Services, cancellationToken, precedingBlock: LastBlockBefore(definition, section, index));
 
                         // A one-for-one replacement keeps the id, so the block can still be referred to by the
                         // name the model already knows.
@@ -203,6 +204,34 @@ protected override async Task<string> ExecuteAsync(PdfToolArguments arguments, P
         }, cancellationToken);
     }
 
+    /// <summary>
+    /// Gets the block that new blocks placed at a position in a section follow, looking back into earlier
+    /// sections when the position is the start of one.
+    /// </summary>
+    /// <param name="definition">The document.</param>
+    /// <param name="section">The section the blocks go into.</param>
+    /// <param name="position">The index in the section the first new block takes.</param>
+    /// <returns>The block before that position, or <see langword="null"/> when there is none.</returns>
+    private static PdfBlockDefinition LastBlockBefore(PdfDocumentDefinition definition, PdfSectionDefinition section, int position)
+    {
+        if (position > 0)
+        {
+            return section.Blocks[position - 1];
+        }
+
+        var sectionIndex = definition.Sections.IndexOf(section);
+
+        for (var index = sectionIndex - 1; index >= 0; index--)
+        {
+            if (definition.Sections[index].Blocks.Count > 0)
+            {
+                return definition.Sections[index].Blocks[^1];
+            }
+        }
+
+        return null;
+    }
+
     private static PdfSectionDefinition ResolveSection(PdfDocumentDefinition definition, PdfToolArguments arguments)
     {
         if (arguments.TryGetElement("new_section", out var element) && element.ValueKind == System.Text.Json.JsonValueKind.Object)

src/Primitives/CrestApps.Core.AI.Documents.Pdf/Tools/PdfBlockPreparer.cs

@@ -17,6 +17,7 @@ internal static class PdfBlockPreparer
 {
     private const int DefaultMaxRows = 1000;
     private const int MaxRowsLimit = 5000;
+    private const string ChartMarkerPrefix = "[chart:";
 
     /// <summary>
     /// Readies blocks for a document.
@@ -26,17 +27,20 @@ internal static class PdfBlockPreparer
     /// <param name="services">The request services, used to reach the tabular workspace.</param>
     /// <param name="cancellationToken">The cancellation token.</param>
     /// <param name="requireChartData">
-    /// Whether a chart block that ends up with nothing to plot fails the call. Blocks the model wrote do: a
-    /// chart left out with only a warning was reported to the user as "the charts are embedded" while the
-    /// file had headings over empty space.
+    /// Whether the blocks are the model's own writing, as opposed to content converted from a file. For them, a
+    /// chart block that ends up with nothing to plot fails the call — a chart left out with only a warning was
+    /// reported to the user as "the charts are embedded" while the file had headings over empty space — and so
+    /// does chart marker text, and a heading that repeats the one right before it is left out.
     /// </param>
+    /// <param name="precedingBlock">The block the new blocks follow in the document, when there is one.</param>
     /// <returns>Notes about data that could not be read as asked.</returns>
     public static async Task<List<string>> PrepareAsync(
         PdfDocumentDefinition definition,
         List<PdfBlockDefinition> blocks,
         IServiceProvider services,
         CancellationToken cancellationToken,
-        bool requireChartData = true)
+        bool requireChartData = true,
+        PdfBlockDefinition precedingBlock = null)
     {
         ArgumentNullException.ThrowIfNull(definition);
 
@@ -49,6 +53,12 @@ public static async Task<List<string>> PrepareAsync(
 
         ReplaceHandWrittenContents(definition, blocks, warnings);
 
+        if (requireChartData)
+        {
+            await ChartsFromMarkerTextAsync(blocks, services);
+            RemoveRepeatedHeadings(blocks, precedingBlock, warnings);
+        }
+
         TabularToolRunner.PreparationResult? tabular = null;
 
         try
@@ -291,6 +301,131 @@ internal static void LiftFlatObject(PdfBlockDefinition block)
         }
     }
 
+    /// <summary>
+    /// Turns a paragraph that is a whole <c>[chart:…]</c> marker into the chart, and refuses any other text that
+    /// carries one.
+    /// </summary>
+    /// <remarks>
+    /// Text is printed as it is written, so a marker in it reached the reader as its characters. Asked to put
+    /// the charts of the conversation in a report, gpt-4.1 wrote <c>[chart:Refer to chart marker above]</c> as a
+    /// paragraph under each chart's heading — a placeholder for a chart it never named — and the file showed
+    /// that line where the chart should be. A marker that holds a chart is drawn; anything else names no chart
+    /// this host can find, so the call is refused with the ways to add one, rather than printing the marker.
+    /// </remarks>
+    /// <param name="blocks">The blocks of the call, changed in place.</param>
+    /// <param name="services">The request services, used to list the charts of the conversation.</param>
+    private static async Task ChartsFromMarkerTextAsync(List<PdfBlockDefinition> blocks, IServiceProvider services)
+    {
+        for (var index = 0; index < blocks.Count; index++)
+        {
+            var block = blocks[index];
+
+            if (block is null)
+            {
+                continue;
+            }
+
+            var type = PdfMigraDocBuilder.NormalizeType(block);
+            var text = block.Text?.Trim();
+
+            if (type is PdfBlockTypes.Paragraph or PdfBlockTypes.Markdown &&
+                text is not null &&
+                text.StartsWith(ChartMarkerPrefix, StringComparison.OrdinalIgnoreCase) &&
+                text.EndsWith(']') &&
+                PdfChartJsReader.FindMarkers(text).Count == 1 &&
+                PdfChartJsReader.TryRead(text, out _))
+            {
+                block.Type = PdfBlockTypes.Chart;
+                block.Chart = new PdfChartDefinition { ChartJs = text };
+                block.Text = null;
+
+                continue;
+            }
+
+            if (type == PdfBlockTypes.Code)
+            {
+                // Code is shown as written on purpose; a marker in it is an example, not a chart.
+                continue;
+            }
+
+            var marked = new[] { block.Text, block.Title }
+                .Concat(block.Items ?? [])
+                .FirstOrDefault(value => value?.Contains(ChartMarkerPrefix, StringComparison.OrdinalIgnoreCase) == true);
+
+            if (marked is null)
+            {
+                continue;
+            }
+
+            var listed = PdfConversationCharts.Describe(await PdfConversationCharts.GetAsync(services));
+
+            throw new PdfToolException(
+                $"Nothing was added: block {index + 1} ({type}) has a chart marker in its text — \"{Excerpt(marked)}\" — and text is printed as written, so the reader would see the marker instead of a chart. " +
+                "Add the chart as its own block: {\"type\": \"chart\", \"conversation_chart\": N} for a chart shown earlier in the conversation, labels and series with the real values, source over uploaded tabular data, or chart_js with the whole [chart:{…}] marker. Never write a marker in text." +
+                (listed is null ? string.Empty : Environment.NewLine + listed));
+        }
+    }
+
+    /// <summary>
+    /// Leaves out a heading that repeats the heading right before it.
+    /// </summary>
+    /// <remarks>
+    /// gpt-4.1 put the document's title heading in <c>create_pdf</c> and then sent the content again starting
+    /// with the same heading, and the report opened with its title twice. Two headings with the same words and
+    /// nothing between them are never two sections, so the second is left out and the model is told. A heading
+    /// used again further on, with content between, is kept.
+    /// </remarks>
+    /// <param name="blocks">The blocks of the call, changed in place.</param>
+    /// <param name="precedingBlock">The block the new blocks follow in the document, when there is one.</param>
+    /// <param name="notes">Receives a note for each heading left out.</param>
+    internal static void RemoveRepeatedHeadings(List<PdfBlockDefinition> blocks, PdfBlockDefinition precedingBlock, List<string> notes)
+    {
+        ArgumentNullException.ThrowIfNull(blocks);
+        ArgumentNullException.ThrowIfNull(notes);
+
+        var previous = precedingBlock;
+
+        for (var index = 0; index < blocks.Count; index++)
+        {
+            var block = blocks[index];
+
+            if (block is null)
+            {
+                continue;
+            }
+
+            if (IsHeading(block) && IsHeading(previous) && string.Equals(HeadingKey(block), HeadingKey(previous), StringComparison.Ordinal))
+            {
+                notes.Add($"Left out the heading \"{block.Text.Trim()}\": it is already the heading before it. Do not send content the document already has; append only what is new.");
+                blocks.RemoveAt(index);
+                index--;
+
+                continue;
+            }
+
+            previous = block;
+        }
+    }
+
+    private static bool IsHeading(PdfBlockDefinition block)
+    {
+        return block is not null &&
+            PdfMigraDocBuilder.NormalizeType(block) == PdfBlockTypes.Heading &&
+            !string.IsNullOrWhiteSpace(block.Text);
+    }
+
+    private static string HeadingKey(PdfBlockDefinition block)
+    {
+        return string.Join(' ', block.Text.Split((char[])null, StringSplitOptions.RemoveEmptyEntries)).ToUpperInvariant();
+    }
+
+    private static string Excerpt(string text)
+    {
+        var trimmed = text.Trim();
+
+        return trimmed.Length <= 80 ? trimmed : trimmed[..80] + "…";
+    }
+
     /// <summary>
     /// Turns an image block that points at a chart — a <c>[chart:…]</c> marker, Chart.js JSON, or a chart of
     /// the conversation — into the chart block it was meant to be.