Recover complete object from concatenated JSON in sampling responses

PrCopilot/cspell.json

@@ -14,6 +14,7 @@
     "nologo",
     "nums",
     "osascript",
+    "parseable",
     "pushback",
     "slnx",
     "unreplied",

PrCopilot/src/PrCopilot/Tools/SamplingHelper.cs

@@ -91,10 +91,53 @@ internal static class SamplingHelper
         }
 
         DebugLogger.Log("Sampling", $"Failed to parse JSON response as {typeof(T).Name} from {blocks.Count} block(s): {lastError}");
-        DebugLogger.Log("Sampling", $"Raw blocks: {string.Join(" || ", blocks).Truncate(500)}");
+        DebugLogger.Log("Sampling", $"Raw blocks: {JoinCapped(blocks, " || ", 2000)}");
         return null;
     }
 
+    /// <summary>
+    /// Join <paramref name="parts"/> with <paramref name="separator"/>, appending at most
+    /// <paramref name="maxLength"/> characters of content (plus a trailing "..." marker when
+    /// truncated, so the result is at most <paramref name="maxLength"/> + 3 characters). This
+    /// keeps the allocation bounded even when individual parts are very large (used for
+    /// diagnostic logging).
+    /// </summary>
+    internal static string JoinCapped(IReadOnlyList<string> parts, string separator, int maxLength)
+    {
+        if (maxLength <= 0)
+            return string.Empty;
+
+        var sb = new StringBuilder(Math.Min(maxLength + 3, 1024));
+        foreach (var part in parts)
+        {
+            // Only append the separator if the following part has room — otherwise the
+            // separator alone could push past maxLength and make remaining negative.
+            var separatorLength = sb.Length > 0 ? separator.Length : 0;
+            if (sb.Length + separatorLength >= maxLength)
+            {
+                // No room for this part — content is being dropped, so mark the truncation.
+                sb.Append("...");
+                break;
+            }
+
+            if (separatorLength > 0)
+                sb.Append(separator);
+
+            var remaining = maxLength - sb.Length;
+            if (part.Length <= remaining)
+            {
+                sb.Append(part);
+            }
+            else
+            {
+                sb.Append(part, 0, remaining).Append("...");
+                break;
+            }
+        }
+
+        return sb.ToString();
+    }
+
     /// <summary>
     /// Attempt to deserialize a single candidate response into <typeparamref name="T"/>,
     /// stripping code fences and escaping stray control characters first.
@@ -104,26 +147,89 @@ private static bool TryDeserialize<T>(string text, out T? parsed, out string? er
     {
         parsed = null;
         error = null;
+
+        // Strip markdown code fences if present (LLMs often wrap JSON in ```json ... ```)
+        // and escape literal control chars inside strings (invalid per RFC 8259).
+        var cleaned = SanitizeJsonControlChars(StripCodeFences(text));
+
         try
         {
-            // Strip markdown code fences if present (LLMs often wrap JSON in ```json ... ```)
-            // and escape literal control chars inside strings (invalid per RFC 8259).
-            var cleaned = SanitizeJsonControlChars(StripCodeFences(text));
             parsed = JsonSerializer.Deserialize<T>(cleaned, _jsonOptions);
-            return parsed != null;
+            if (parsed != null)
+                return true;
         }
         catch (JsonException ex)
         {
             error = ex.Message;
-            return false;
         }
         catch (FileNotFoundException ex)
         {
             // .NET single-file publishing can throw FileNotFoundException instead of JsonException
             // when satellite assemblies for JSON error messages are missing.
             error = $"[{ex.GetType().Name}] {ex.Message}";
-            return false;
         }
+
+        // Fallback: the text may contain a truncated false-start object followed by the
+        // complete object, both in one block (e.g. {"a": "part\n{"a": "whole"}).
+        // A direct parse fails, so extract the largest parseable object instead.
+        if (TryExtractBestObject<T>(cleaned, out parsed))
+        {
+            error = null;
+            return true;
+        }
+
+        return false;
+    }
+
+    /// <summary>
+    /// Scan the text for candidate object starts (every '{', which may include nested or
+    /// non-top-level positions) and return the value that deserializes into
+    /// <typeparamref name="T"/> while consuming the most bytes. This recovers the complete
+    /// object when the response also contains a truncated false-start object that a direct
+    /// parse chokes on; candidate starts that aren't a valid object simply fail to parse.
+    /// </summary>
+    private static bool TryExtractBestObject<T>(string text, out T? parsed) where T : class
+    {
+        parsed = null;
+        long bestConsumed = -1;
+
+        for (int i = 0; i < text.Length; i++)
+        {
+            if (text[i] != '{')
+                continue;
+
+            try
+            {
+                var bytes = Encoding.UTF8.GetBytes(text[i..]);
+                var reader = new Utf8JsonReader(bytes);
+                var candidate = JsonSerializer.Deserialize<T>(ref reader, _jsonOptions);
+
+                // Deserialize(ref reader) reads exactly one value and ignores anything
+                // after it, so trailing content (a second object, prose, etc.) is fine.
+                if (candidate != null && reader.BytesConsumed > bestConsumed)
+                {
+                    bestConsumed = reader.BytesConsumed;
+                    parsed = candidate;
+
+                    // If this object consumed the whole remaining suffix, no later start
+                    // (which has fewer bytes to work with) can beat it — stop early.
+                    if (reader.BytesConsumed == bytes.Length)
+                        break;
+                }
+            }
+            catch (JsonException)
+            {
+                // Not a valid object start (e.g. a '{' inside a string, or a truncated
+                // false-start) — expected while scanning; try the next candidate.
+            }
+            catch (FileNotFoundException)
+            {
+                // Single-file publishing can surface JSON parse errors as this instead;
+                // treat the same as an invalid candidate.
+            }
+        }
+
+        return parsed != null;
     }
 
     /// <summary>

PrCopilot/tests/PrCopilot.Tests/SamplingHelperTests.cs

@@ -399,6 +399,88 @@ public async Task SampleStructuredAsync_MultipleBlocks_OneObjectSplitAcrossBlock
         Assert.Equal("clarify", result.RecommendationType);
     }
 
+    [Fact]
+    public async Task SampleStructuredAsync_SingleBlockTwoObjects_UsesCompleteObject()
+    {
+        // The exact PR 16255 failure: a truncated false-start object and the complete
+        // object arrive concatenated in ONE content block (blocks=1).
+        var truncated = "{\"explanation\": \"The reviewer notes that deleting the analyzer means AZC0005 is no longer enforced";
+        var complete = "{\"explanation\": \"The reviewer flags missing test coverage.\", \"recommendation\": \"Push back; the removal is intentional and in scope.\", \"recommendationType\": \"pushback\"}";
+        var server = new FakeSamplingMcpServer(truncated + "\n" + complete);
+
+        var result = await SamplingHelper.SampleStructuredAsync<CommentExplanationTestModel>(
+            server, "system", "user", maxTokens: 100);
+
+        Assert.NotNull(result);
+        Assert.Equal("The reviewer flags missing test coverage.", result!.Explanation);
+        Assert.Contains("intentional", result.Recommendation);
+        Assert.Equal("pushback", result.RecommendationType);
+    }
+
+    [Fact]
+    public async Task SampleStructuredAsync_TrailingProseAfterObject_StillParses()
+    {
+        // Complete object followed by trailing prose the model tacked on.
+        var response = "{\"explanation\": \"done\", \"recommendation\": \"fix\", \"recommendationType\": \"implement\"}\n\nLet me know if you'd like more detail.";
+        var server = new FakeSamplingMcpServer(response);
+
+        var result = await SamplingHelper.SampleStructuredAsync<CommentExplanationTestModel>(
+            server, "system", "user", maxTokens: 100);
+
+        Assert.NotNull(result);
+        Assert.Equal("done", result!.Explanation);
+        Assert.Equal("implement", result.RecommendationType);
+    }
+
+    [Fact]
+    public void JoinCapped_ShortInput_JoinsFully()
+    {
+        var parts = new List<string> { "one", "two", "three" };
+        var result = SamplingHelper.JoinCapped(parts, " || ", 2000);
+        Assert.Equal("one || two || three", result);
+    }
+
+    [Fact]
+    public void JoinCapped_HugeInput_BoundsAllocation()
+    {
+        var parts = new List<string> { new string('a', 5000), new string('b', 5000) };
+        var result = SamplingHelper.JoinCapped(parts, " || ", 2000);
+
+        // Never materializes the full 10k+ join — bounded to the cap plus the "..." marker.
+        Assert.True(result.Length <= 2003, $"length was {result.Length}");
+        Assert.EndsWith("...", result);
+        Assert.StartsWith("aaa", result);
+    }
+
+    [Fact]
+    public void JoinCapped_FirstPartLeavesLessThanSeparator_DoesNotThrow()
+    {
+        // Regression: the first part leaves fewer than separator-length chars before the cap
+        // (1999 chars, 2000 cap, 4-char separator). Previously the separator was appended
+        // unconditionally, making remaining negative and throwing ArgumentOutOfRangeException
+        // in the failure-logging path.
+        var parts = new List<string> { new string('a', 1999), new string('b', 100) };
+        var result = SamplingHelper.JoinCapped(parts, " || ", 2000);
+
+        Assert.True(result.Length <= 2003, $"length was {result.Length}");
+        Assert.StartsWith("aaa", result);
+        Assert.DoesNotContain("b", result);
+        // Content was dropped between parts — the truncation marker must be present.
+        Assert.EndsWith("...", result);
+    }
+
+    [Fact]
+    public void JoinCapped_TruncatesBetweenParts_AppendsMarker()
+    {
+        // The second part doesn't fit even the separator, so it's dropped between parts.
+        var parts = new List<string> { new string('a', 8), new string('b', 8) };
+        var result = SamplingHelper.JoinCapped(parts, " || ", 10);
+
+        Assert.StartsWith("aaa", result);
+        Assert.DoesNotContain("b", result);
+        Assert.EndsWith("...", result);
+    }
+
     private class TestResponse
     {
         public string? Name { get; set; }