Fix repository review findings

AgentCommunicationFaultTolerance.AgentFramework/ReliableChannel.cs

@@ -24,17 +24,28 @@ public sealed class FlakyTransport(int seed, double lossRate, double duplicateRa
 public sealed class Inbox
 {
     readonly Dictionary<string, string> handled = new(StringComparer.Ordinal);
+    readonly object gate = new();
 
-    public IReadOnlyDictionary<string, string> Handled => handled;
+    public IReadOnlyDictionary<string, string> Handled
+    {
+        get
+        {
+            lock (gate) return new Dictionary<string, string>(handled, StringComparer.Ordinal);
+        }
+    }
 
     /// Returns the effect's result and whether this was a replay rather than a first delivery.
     public (string Result, bool Duplicate) Handle(Message message, Func<Message, string> effect)
     {
-        if (handled.TryGetValue(message.Id, out var existing)) return (existing, true);
+        // ponytail: one global lock; use per-message locks if unrelated effects need parallel throughput.
+        lock (gate)
+        {
+            if (handled.TryGetValue(message.Id, out var existing)) return (existing, true);
 
-        var result = effect(message);
-        handled[message.Id] = result;
-        return (result, false);
+            var result = effect(message);
+            handled[message.Id] = result;
+            return (result, false);
+        }
     }
 }
 
@@ -84,6 +95,9 @@ public async Task<Delivery> SendAsync(Message message, Func<Message, string> eff
     /// The step people skip. Retries and dead-letters make each message's fate correct; only a
     /// reconciliation pass makes the CONVERSATION correct - it is where you find out that agent B
     /// is missing the one message agent A believes it sent.
-    public static IReadOnlyList<string> Reconcile(IEnumerable<Message> sent, Inbox inbox) =>
-        [.. sent.Select(m => m.Id).Where(id => !inbox.Handled.ContainsKey(id))];
+    public static IReadOnlyList<string> Reconcile(IEnumerable<Message> sent, Inbox inbox)
+    {
+        var handled = inbox.Handled;
+        return [.. sent.Select(m => m.Id).Where(id => !handled.ContainsKey(id))];
+    }
 }

AgenticPatterns.Tests/CasePartitionTests.cs

@@ -38,6 +38,15 @@ public void EmptyReviewedByIsAlsoAwaitingReview()
         Assert.Single(awaitingReview);
     }
 
+    [Fact]
+    public void WhitespaceReviewedByIsAlsoAwaitingReview()
+    {
+        var (evaluated, awaitingReview) = CasePartition.Partition([Case("blank", reviewedBy: "  \t")]);
+
+        Assert.Empty(evaluated);
+        Assert.Single(awaitingReview);
+    }
+
     [Fact]
     public void MixedCorpusSplitsCorrectly()
     {

AgenticPatterns.Tests/NewContextPatternTests.cs

@@ -196,6 +196,15 @@ public void RecencyDecaysWithAge()
         Assert.True(scored[0].Recency > scored[1].Recency);
     }
 
+    [Fact]
+    public void DayOldRecencyMatchesTheDocumentedDecay()
+    {
+        var scored = EpisodicRetrieval.Score(
+            [new("ep-1", "memory", Now.AddDays(-1), 0.5, "t")], "unrelated", Now);
+
+        Assert.InRange(scored.Single().Recency, 0.88, 0.89);
+    }
+
     [Fact]
     public void OnlyTopicsOverTheThresholdConsolidate()
     {

AgenticPatterns.Tests/NewProductionControlTests.cs

@@ -30,6 +30,47 @@ public void AWellFormedChainPasses() =>
             new Step("file_expense", ["total"], "receipt", "text")
         ], Tools));
 
+    [Fact]
+    public void ToolContractsRejectWrongArityAndOutputTypes()
+    {
+        var errors = DataFlowPlan.Validate(
+        [
+            new Step("fetch_email", [], "email", "text"),
+            new Step("extract_total", [], "total", "decimal"),
+            new Step("file_expense", ["total"], "receipt", "text")
+        ], Tools);
+
+        Assert.Contains(errors, error => error.Message.Contains("produces 'untrusted_text'"));
+        Assert.Contains(errors, error => error.Message.Contains("expects 1 argument"));
+    }
+
+    [Fact]
+    public void ConsumerInputTypesMustMatchTheirProducer()
+    {
+        var errors = DataFlowPlan.Validate(
+        [
+            new Step("fetch_email", [], "email", "untrusted_text"),
+            new Step("extract_total", ["email"], "total", "decimal"),
+            new Step("file_expense", ["email"], "receipt", "text")
+        ], Tools);
+
+        Assert.Contains(errors, error => error.Message.Contains("expected 'decimal'"));
+    }
+
+    [Fact]
+    public void APlanMustHaveExactlyOneSideEffectingSink()
+    {
+        var errors = DataFlowPlan.Validate(
+        [
+            new Step("fetch_email", [], "email", "untrusted_text"),
+            new Step("extract_total", ["email"], "total", "decimal"),
+            new Step("file_expense", ["total"], "receipt-1", "text"),
+            new Step("file_expense", ["total"], "receipt-2", "text")
+        ], Tools);
+
+        Assert.Contains(errors, error => error.Message.Contains("exactly one 'file_expense'"));
+    }
+
     [Fact]
     public void ReassigningAVariableIsRejected() =>
         Assert.NotEmpty(DataFlowPlan.Validate(
@@ -186,6 +227,29 @@ public async Task ADuplicateDeliveryRunsTheEffectOnce()
         Assert.Equal(1, runs);
     }
 
+    [Fact]
+    public async Task ConcurrentDuplicateDeliveriesRunTheEffectOnce()
+    {
+        const int callers = 8;
+        using var start = new Barrier(callers);
+        var runs = 0;
+        var inbox = new Inbox();
+
+        await Task.WhenAll(Enumerable.Range(0, callers).Select(_ => Task.Run(() =>
+        {
+            start.SignalAndWait();
+            inbox.Handle(Message("M1"), _ =>
+            {
+                Interlocked.Increment(ref runs);
+                Thread.Sleep(50);
+                return "done";
+            });
+        })));
+
+        Assert.Equal(1, runs);
+        Assert.Single(inbox.Handled);
+    }
+
     [Fact]
     public async Task ARetriedMessageStillOnlyRunsTheEffectOnce()
     {

AgenticPatterns.Tests/ProductionControlTests.cs

@@ -570,22 +570,42 @@ public void RedactionCopiesResponseMetadata()
     }
 
     [Fact]
-    public void TruncateTrimsOnlyTheLastTextContent()
+    public void TruncateKeepsThePrefixAndCountsTheMarkerWithinTheLimit()
     {
         var functionCall = new FunctionCallContent("c1", "lookup", null);
         var response = new ChatResponse([
             new ChatMessage(ChatRole.Assistant, [new TextContent("first ten.")]),
-            new ChatMessage(ChatRole.Assistant, [functionCall, new TextContent("second block of text")])
+            new ChatMessage(ChatRole.Assistant,
+                [functionCall, new TextContent("second block of text that keeps going for much longer")])
         ]);
 
-        // 10 ("first ten.") + budget 5 left for the last TextContent out of a 15-character cap.
-        var truncated = GuardRails.Truncate(response, maxCharacters: 15);
+        var truncated = GuardRails.Truncate(response, maxCharacters: 50);
 
         Assert.Equal("first ten.", ((TextContent)truncated.Messages[0].Contents[0]).Text);
         Assert.Same(functionCall, truncated.Messages[1].Contents[0]);
         var lastText = ((TextContent)truncated.Messages[1].Contents[1]).Text;
-        Assert.StartsWith("secon", lastText);
+        Assert.StartsWith("second", lastText);
         Assert.Contains("[Response truncated for safety.]", lastText);
+        Assert.True(truncated.Messages.Sum(message =>
+            message.Contents.OfType<TextContent>().Sum(text => text.Text.Length)) <= 50);
+    }
+
+    [Fact]
+    public void TruncateCapsEarlierTextAndClearsLaterText()
+    {
+        var functionCall = new FunctionCallContent("c1", "lookup", null);
+        var response = new ChatResponse([
+            new ChatMessage(ChatRole.Assistant, new string('x', 100)),
+            new ChatMessage(ChatRole.Assistant, [functionCall, new TextContent("later")])
+        ]);
+
+        var truncated = GuardRails.Truncate(response, maxCharacters: 50);
+
+        Assert.Equal(50, truncated.Messages.Sum(message =>
+            message.Contents.OfType<TextContent>().Sum(text => text.Text.Length)));
+        Assert.Contains("[Response truncated for safety.]", truncated.Messages[0].Text);
+        Assert.Equal("", truncated.Messages[1].Text);
+        Assert.Same(functionCall, truncated.Messages[1].Contents[0]);
     }
 
     [Fact]

AgenticPatterns.Tests/ProductionControlsPhaseTwoTests.cs

@@ -107,6 +107,21 @@ await Assert.ThrowsAsync<InvalidOperationException>(() =>
             replay.GetResponseAsync([new ChatMessage(ChatRole.User, "changed")]));
     }
 
+    [Fact]
+    public void RequestHashIncludesGenerationOptions()
+    {
+        var messages = new[] { new ChatMessage(ChatRole.User, "same prompt") };
+        var original = TraceStore.HashMessages(messages,
+            new ChatOptions { Instructions = "v1", MaxOutputTokens = 100 }, TracePrivacyMode.FullContent);
+        var changedInstructions = TraceStore.HashMessages(messages,
+            new ChatOptions { Instructions = "v2", MaxOutputTokens = 100 }, TracePrivacyMode.FullContent);
+        var changedLimit = TraceStore.HashMessages(messages,
+            new ChatOptions { Instructions = "v1", MaxOutputTokens = 500 }, TracePrivacyMode.FullContent);
+
+        Assert.NotEqual(original, changedInstructions);
+        Assert.NotEqual(original, changedLimit);
+    }
+
     [Fact]
     public async Task TraceFileRoundTrips()
     {

DualLlm.AgentFramework/DataFlow.cs

@@ -15,29 +15,57 @@ public sealed record PlanError(string Step, string Message);
 
 public static class DataFlowPlan
 {
+    private static readonly IReadOnlyDictionary<string, (string[] Inputs, string Output)> Contracts =
+        new Dictionary<string, (string[] Inputs, string Output)>(StringComparer.Ordinal)
+        {
+            ["fetch_email"] = ([], "untrusted_text"),
+            ["extract_total"] = (["untrusted_text"], "decimal"),
+            ["file_expense"] = (["decimal"], "text")
+        };
+
     /// The plan is written by the privileged model, which has seen only the user's instruction -
     /// but "privileged" describes what it was shown, not that its output is trusted. Validate the
     /// whole plan before a single step runs.
     public static IReadOnlyList<PlanError> Validate(IReadOnlyList<Step> steps,
         IReadOnlySet<string> allowedTools)
     {
         var errors = new List<PlanError>();
-        var produced = new HashSet<string>(StringComparer.Ordinal);
+        var produced = new Dictionary<string, string>(StringComparer.Ordinal);
 
         foreach (var step in steps)
         {
             if (!allowedTools.Contains(step.Tool))
                 errors.Add(new PlanError(step.Tool, $"tool '{step.Tool}' is not allowed"));
 
-            foreach (var arg in step.Args)
-                if (!produced.Contains(arg))
+            var hasContract = Contracts.TryGetValue(step.Tool, out var contract);
+            if (!hasContract)
+                errors.Add(new PlanError(step.Tool, $"tool '{step.Tool}' has no declared data-flow contract"));
+            else if (step.Args.Length != contract.Inputs.Length)
+                errors.Add(new PlanError(step.Tool,
+                    $"tool '{step.Tool}' expects {contract.Inputs.Length} argument(s), got {step.Args.Length}"));
+
+            for (var i = 0; i < step.Args.Length; i++)
+            {
+                var arg = step.Args[i];
+                if (!produced.TryGetValue(arg, out var actualType))
                     errors.Add(new PlanError(step.Tool,
                         $"argument '{arg}' is not a variable produced by an earlier step"));
+                else if (hasContract && i < contract.Inputs.Length && actualType != contract.Inputs[i])
+                    errors.Add(new PlanError(step.Tool,
+                        $"argument '{arg}' has type '{actualType}', expected '{contract.Inputs[i]}'"));
+            }
 
-            if (!produced.Add(step.Produces))
+            if (hasContract && step.ProducesType != contract.Output)
+                errors.Add(new PlanError(step.Tool,
+                    $"tool '{step.Tool}' produces '{contract.Output}', not '{step.ProducesType}'"));
+
+            if (!produced.TryAdd(step.Produces, step.ProducesType))
                 errors.Add(new PlanError(step.Tool, $"variable '{step.Produces}' is assigned twice"));
         }
 
+        if (steps.Count(step => step.Tool == "file_expense") != 1)
+            errors.Add(new PlanError("file_expense", "plan must contain exactly one 'file_expense' step"));
+
         return errors;
     }
 

EvaluationAndMonitoring.AgentFramework/TraceReplay.cs

@@ -212,7 +212,22 @@ public static string CanonicalOptions(ChatOptions? options)
         var tools = string.Join(";", options?.Tools?.Select(tool => tool is AIFunctionDeclaration function
             ? $"{function.Name}:{function.JsonSchema.GetRawText()}"
             : $"{tool.Name}:{tool.Description}") ?? []);
-        return $"model:{options?.ModelId}|temperature:{options?.Temperature}|format:{options?.ResponseFormat}|tools:{tools}";
+        var toolMode = options?.ToolMode is RequiredChatToolMode required
+            ? $"Required:{required.RequiredFunctionName}"
+            : options?.ToolMode?.GetType().Name ?? "";
+        var additionalProperties = options?.AdditionalProperties is { } props
+            ? JsonSerializer.Serialize(props.OrderBy(p => p.Key, StringComparer.Ordinal))
+            : "";
+
+        return string.Join('|',
+            $"model:{options?.ModelId}", $"temperature:{options?.Temperature}", $"format:{options?.ResponseFormat}",
+            $"tools:{tools}", $"toolMode:{toolMode}", $"allowMultipleToolCalls:{options?.AllowMultipleToolCalls}",
+            $"instructions:{options?.Instructions}", $"maxOutputTokens:{options?.MaxOutputTokens}",
+            $"topP:{options?.TopP}", $"topK:{options?.TopK}", $"seed:{options?.Seed}",
+            $"stopSequences:{string.Join(",", options?.StopSequences ?? [])}",
+            $"frequencyPenalty:{options?.FrequencyPenalty}", $"presencePenalty:{options?.PresencePenalty}",
+            $"reasoningEffort:{options?.Reasoning?.Effort}", $"reasoningOutput:{options?.Reasoning?.Output}",
+            $"additionalProperties:{additionalProperties}");
     }
 
     private static string Redact(string value) => Regex.Replace(

GuardRails.AgentFramework/GuardRails.cs

@@ -18,10 +18,8 @@ public static ChatResponse Redact(ChatResponse response) =>
         WithMessages(response, RedactMessages(response.Messages));
 
     /// <summary>
-    /// Caps the response's total text at <paramref name="maxCharacters"/>. Only the last
-    /// <see cref="TextContent"/> in the response absorbs the cut — earlier text and every
-    /// non-text content item are left untouched. See <see cref="TruncateMessages"/> for the
-    /// exact budget rule.
+    /// Caps the response's total text at <paramref name="maxCharacters"/>, including the
+    /// truncation marker. Non-text content items are left untouched.
     /// </summary>
     public static ChatResponse Truncate(ChatResponse response, int maxCharacters) =>
         WithMessages(response, TruncateMessages(response.Messages, maxCharacters));
@@ -41,46 +39,40 @@ internal static ChatMessage RedactMessage(ChatMessage message) =>
     /// <summary>
     /// Truncates a list of messages to at most <paramref name="maxCharacters"/> characters of
     /// total text. "Total text" is the sum of every <see cref="TextContent.Text"/> length across
-    /// every message. When that sum exceeds the limit, only the last message's last
-    /// <see cref="TextContent"/> is shortened — down to whatever budget remains once every other
-    /// text item is counted — and a truncation marker is appended; everything before it, and
-    /// every non-text content item, is untouched. Returns the original list unchanged (same
-    /// reference) when already under budget, so callers can detect a no-op with
+    /// every message. When that sum exceeds the limit, the response prefix is kept, a truncation
+    /// marker is appended within the budget, and later text is cleared. Non-text content is
+    /// untouched. Returns the original list unchanged (same reference) when already under budget,
+    /// so callers can detect a no-op with
     /// <see cref="object.ReferenceEquals(object?, object?)"/>.
     /// </summary>
     internal static IList<ChatMessage> TruncateMessages(IList<ChatMessage> messages, int maxCharacters)
     {
+        ArgumentOutOfRangeException.ThrowIfNegative(maxCharacters);
+
         var totalLength = messages.Sum(m => m.Contents.OfType<TextContent>().Sum(t => t.Text.Length));
         if (totalLength <= maxCharacters)
             return messages;
 
-        var lastMessageIndex = -1;
-        var lastTextIndex = -1;
-        for (var i = messages.Count - 1; i >= 0 && lastMessageIndex < 0; i--)
+        var marker = TruncationSuffix[..Math.Min(TruncationSuffix.Length, maxCharacters)];
+        var remaining = maxCharacters - marker.Length;
+        var truncated = false;
+        return messages.Select(message =>
         {
-            var contents = messages[i].Contents;
-            for (var j = contents.Count - 1; j >= 0; j--)
+            var contents = message.Contents.Select(content =>
             {
-                if (contents[j] is not TextContent) continue;
-                lastMessageIndex = i;
-                lastTextIndex = j;
-                break;
-            }
-        }
-
-        // No TextContent anywhere in the response — nothing this function is responsible for can trim.
-        if (lastMessageIndex < 0)
-            return messages;
-
-        var lastText = (TextContent)messages[lastMessageIndex].Contents[lastTextIndex];
-        var budget = Math.Max(0, maxCharacters - (totalLength - lastText.Text.Length));
-        var truncatedText = (budget < lastText.Text.Length ? lastText.Text[..budget] : lastText.Text) + TruncationSuffix;
+                if (content is not TextContent text) return content;
+                if (truncated) return new TextContent("");
+                if (text.Text.Length <= remaining)
+                {
+                    remaining -= text.Text.Length;
+                    return content;
+                }
 
-        var newContents = messages[lastMessageIndex].Contents.ToList();
-        newContents[lastTextIndex] = new TextContent(truncatedText);
-        var newMessages = messages.ToList();
-        newMessages[lastMessageIndex] = CloneWithContents(messages[lastMessageIndex], newContents);
-        return newMessages;
+                truncated = true;
+                return new TextContent(text.Text[..remaining] + marker);
+            }).ToList();
+            return CloneWithContents(message, contents);
+        }).ToList();
     }
 
     // Core mapper: rewrites only TextContent items (via `transform`) in a message's Contents,