diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..fb71105 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,22 @@ +name: Unit Tests + +on: + push: + branches: [main, master] + paths: + - 'Source/**' + - 'Tests/**' + - 'ArchTests/**' + - '.github/workflows/test.yml' + pull_request: + branches: [main, master] + paths: + - 'Source/**' + - 'Tests/**' + - 'ArchTests/**' + - '.github/workflows/test.yml' + +jobs: + test: + uses: RimWorld-RimMind-Mod/Workflows/.github/workflows/test.yml@main + secrets: inherit diff --git a/Source/Advisor/AdvisorCooldownCalculator.cs b/Source/Advisor/AdvisorCooldownCalculator.cs index 8b5f10b..2a05fbb 100644 --- a/Source/Advisor/AdvisorCooldownCalculator.cs +++ b/Source/Advisor/AdvisorCooldownCalculator.cs @@ -44,17 +44,56 @@ public static int CalculateDynamicCooldown( return Math.Max(safeMin, dynamicCooldown); } + public const float MinActivityCooldownMultiplier = 0.35f; + public const float MaxActivityCooldownMultiplier = 3.5f; + + /// + /// Scales a cooldown by Core's ActivityFrequencyScale (higher scale = shorter cooldown), + /// clamped to [0.35x, 3.5x]. Non-positive or tiny scales leave the cooldown unchanged. + /// + public static int ApplyActivityFrequencyScale(int cooldownTicks, float activityFrequencyScale) + => RimMind.Application.Common.Helpers.ActivityCooldownScaler.Scale(cooldownTicks, activityFrequencyScale); + + /// + /// The single effective Advisor cooldown: danger-adapted base cooldown, scaled by the + /// activity frequency, never below the crisis safety floor. Every readiness check (scan, + /// CanRequestAdvice, gizmo/debug display) must derive from this value. + /// + public static int CalculateEffectiveCooldown( + int baseCooldownTicks, + bool isMapHighDanger, + bool isPawnInCrisis, + bool isPawnInExtremeBreakRisk, + float activityFrequencyScale, + int minCooldownTicks = MinCrisisCooldownTicks) + { + int dynamicCooldown = CalculateDynamicCooldown( + baseCooldownTicks, + isMapHighDanger, + isPawnInCrisis, + isPawnInExtremeBreakRisk, + minCooldownTicks); + int scaled = ApplyActivityFrequencyScale(dynamicCooldown, activityFrequencyScale); + return Math.Max(Math.Max(600, minCooldownTicks), scaled); + } + + /// Ticks left until the next request is allowed (0 when ready). + public static int CooldownTicksLeft(int effectiveCooldownTicks, int nowTicks, int lastRequestTick) + => Math.Max(0, effectiveCooldownTicks - (nowTicks - lastRequestTick)); + #if !RIMMIND_ADVISOR_TESTS /// - /// Evaluates pawn and map state to determine the dynamic cooldown. + /// Evaluates pawn and map state to determine the effective cooldown + /// (including Core's ActivityFrequencyScale). /// public static int CalculateCooldownForPawn(Pawn pawn, RimMindAdvisorSettings settings) { if (settings == null) return 30000; + float activityScale = RimMind.Presentation.Api.RimMindAPI.Settings.ActivityFrequencyScale; if (pawn == null) - return settings.requestCooldownTicks; + return CalculateEffectiveCooldown(settings.requestCooldownTicks, false, false, false, activityScale); bool isMapHighDanger = false; try @@ -98,11 +137,12 @@ public static int CalculateCooldownForPawn(Pawn pawn, RimMindAdvisorSettings set // Defensive fallback } - return CalculateDynamicCooldown( + return CalculateEffectiveCooldown( settings.requestCooldownTicks, isMapHighDanger, isPawnInCrisis, - isBreakRisk); + isBreakRisk, + activityScale); } #endif } diff --git a/Source/Advisor/AdvisorCycleCoordinator.cs b/Source/Advisor/AdvisorCycleCoordinator.cs index 6ed6801..0d4e834 100644 --- a/Source/Advisor/AdvisorCycleCoordinator.cs +++ b/Source/Advisor/AdvisorCycleCoordinator.cs @@ -48,7 +48,10 @@ public AdvisorCycleCoordinator( public int LastRequestTick { get => _lastRequestTick; set => _lastRequestTick = value; } public AdvisorTaskDriver? TaskDriver => _taskDriver; public int CooldownTicksLeft => - Math.Max(0, AdvisorCooldownCalculator.CalculateCooldownForPawn(_pawn, Settings) - (Find.TickManager.TicksGame - _lastRequestTick)); + AdvisorCooldownCalculator.CooldownTicksLeft( + AdvisorCooldownCalculator.CalculateCooldownForPawn(_pawn, Settings), + Find.TickManager.TicksGame, + _lastRequestTick); private RimMindAdvisorSettings Settings => _settings(); @@ -95,8 +98,8 @@ public void ForceRequestAdvice() } _lastRequestTick = -9999; - RimMindAPI.ClearModCooldown("Advisor"); - Log.Message("[RimMind-Advisor] ForceRequest: Core-layer cooldown cleared (Advisor), sending request..."); + RimMindAPI.ClearModCooldown(AdvisorRequestIds.RequestModId); + Log.Message($"[RimMind-Advisor] ForceRequest: Core-layer cooldown cleared ({AdvisorRequestIds.RequestModId}), sending request..."); RequestAdvice(Settings); } @@ -174,17 +177,25 @@ private void OnAdviceReceived( continue; } - if (AdvisorToolRiskResolver.IsAutonomyForbidden(_pawn, toolCall.Name)) + var preflightArgs = new ToolCallArgs + { + ToolCallId = toolCall.Id ?? string.Empty, + ToolName = toolCall.Name ?? string.Empty, + ArgumentsJson = string.IsNullOrWhiteSpace(toolCall.Arguments) ? "{}" : toolCall.Arguments, + PawnId = _pawn.thingIDNumber + }; + + if (AdvisorToolRiskResolver.IsAutonomyForbidden(_pawn, toolCall.Name, preflightArgs)) { var blockedReason = ExtractToolCallReason(toolCall) ?? toolCall.Name; - RecordToolHistory(toolCall.Name, blockedReason, "blocked"); + RecordToolHistory(toolCall.Name!, blockedReason, "blocked"); RimMindErrors.Warn( $"[RimMind-Advisor] Tool '{toolCall.Name}' blocked by autonomy policy for {_pawn.Name.ToStringShort}."); continue; } - var riskLevel = AdvisorToolRiskResolver.Resolve(toolCall.Name); - if (ShouldDeferForApproval(toolCall, riskLevel)) + var riskLevel = AdvisorToolRiskResolver.Resolve(toolCall.Name, preflightArgs); + if (ShouldDeferForApproval(toolCall, riskLevel, preflightArgs)) { deferredCalls.Add((toolCall, riskLevel)); } @@ -198,11 +209,11 @@ private void OnAdviceReceived( if (deferredCalls.Count > 1) { - SubmitCabinetProposalForApproval(deferredCalls, taskDriver, requestCycle); + SubmitCabinetProposalForApproval(deferredCalls, taskDriver, requestCycle, response.RequestId); } else if (deferredCalls.Count == 1) { - SubmitToolCallForApproval(deferredCalls[0].toolCall, deferredCalls[0].riskLevel, taskDriver, requestCycle); + SubmitToolCallForApproval(deferredCalls[0].toolCall, deferredCalls[0].riskLevel, taskDriver, requestCycle, response.RequestId); } if (approvedCalls.Count > 0) @@ -367,9 +378,18 @@ private void CompleteRequestCycle() } } - private bool ShouldDeferForApproval(ClientStructuredToolCall call, RiskLevel riskLevel) => + private bool ShouldDeferForApproval(ClientStructuredToolCall call, RiskLevel riskLevel, ToolCallArgs? args = null) => ShouldDeferForApproval(riskLevel, call.Arguments) || - AdvisorToolRiskResolver.RequiresAutonomyApproval(_pawn, call.Name ?? string.Empty); + AdvisorToolRiskResolver.RequiresAutonomyApproval( + _pawn, + call.Name ?? string.Empty, + args ?? new ToolCallArgs + { + ToolCallId = call.Id ?? string.Empty, + ToolName = call.Name ?? string.Empty, + ArgumentsJson = string.IsNullOrWhiteSpace(call.Arguments) ? "{}" : call.Arguments, + PawnId = _pawn.thingIDNumber + }); private bool ShouldDeferForApproval(RiskLevel riskLevel, string? arguments) => AdvisorApprovalPolicy.RequiresApproval( @@ -382,7 +402,8 @@ private bool SubmitToolCallForApproval( ClientStructuredToolCall toolCall, RiskLevel riskLevel, AdvisorTaskDriver taskDriver, - AdvisorRequestCycleState requestCycle) + AdvisorRequestCycleState requestCycle, + string? requestId = null) { var reason = ExtractToolCallReason(toolCall) ?? toolCall.Name; var arguments = ParseToolCallArguments(toolCall.Arguments); @@ -426,7 +447,7 @@ private bool SubmitToolCallForApproval( var approvedCall = StripPreApproval(toolCall); var calls = new List { approvedCall }; - var results = ExecuteToolCallsSafely(calls, toolCall.Id, playerApproved: true); + var results = ExecuteToolCallsSafely(calls, requestId ?? toolCall.Id, playerApproved: true); taskDriver.BroadcastDecisionExecuted(toolCall.Name, reason); foreach (var result in results) { @@ -492,7 +513,8 @@ private bool SubmitToolCallForApproval( private bool SubmitCabinetProposalForApproval( IReadOnlyList<(ClientStructuredToolCall toolCall, RiskLevel riskLevel)> deferredCalls, AdvisorTaskDriver taskDriver, - AdvisorRequestCycleState requestCycle) + AdvisorRequestCycleState requestCycle, + string? requestId = null) { if (deferredCalls == null || deferredCalls.Count == 0) return false; @@ -579,7 +601,7 @@ private bool SubmitCabinetProposalForApproval( var selectedCalls = indicesToRun.Select(i => callsToExecute[i]).ToList(); var selectedReasons = indicesToRun.Select(i => !string.IsNullOrWhiteSpace(reasonsToRecord[i]) ? reasonsToRecord[i] : "Cabinet Plan").ToList(); var preApprovedCalls = selectedCalls.Select(StripPreApproval).ToList(); - var results = ExecuteToolCallsSafely(preApprovedCalls, null, playerApproved: true); + var results = ExecuteToolCallsSafely(preApprovedCalls, requestId, playerApproved: true); for (int i = 0; i < selectedCalls.Count; i++) { var call = selectedCalls[i]; @@ -702,6 +724,28 @@ private void QueueFeedbackBatch( return reason; var readableTool = FormatHumanReadableToolName(toolCall.Name); + + try + { + var summaryKeys = RimMindAPI.Tools.FindById(toolCall.Name ?? string.Empty)?.Definition?.Manifest?.SummaryArgKeys; + if (summaryKeys != null) + { + for (int i = 0; i < summaryKeys.Count; i++) + { + var key = summaryKeys[i]; + if (!string.IsNullOrWhiteSpace(key) && + arguments.TryGetValue(key, out var summaryVal) && + !summaryVal.NullOrEmpty()) + { + return $"{readableTool}: {FormatHumanReadableToolName(summaryVal)}"; + } + } + } + } + catch + { + } + string[] detailKeys = { "building_def", "recipe_def", "zone_type", "work_type", @@ -726,44 +770,15 @@ private List ExecuteToolCallsSafely( string? traceId, bool playerApproved = false) { - try - { - // Approval travels via ToolCallArgs.IsPlayerApproved (trusted channel), never via LLM JSON. - var executionTask = _toolExecutor.ExecuteAsync( - calls, - $"NPC-{_pawn.thingIDNumber}", - traceId, - CancellationToken.None, - playerApproved); - - if (!executionTask.IsCompleted) - { - RimMind.Application.Common.Threading.SynchronousTaskGate.ObserveFault(executionTask); - RimMindErrors.Warn( - "[RimMind-Advisor][ToolCall][MainThreadWait] " + - $"Tool execution did not complete synchronously on main thread for {_pawn.Name.ToStringShort} " + - $"(trace={traceId ?? "unknown"}, tools={calls.Count}); failing fast to avoid main-thread freeze."); - return calls - .Select(call => ToolResult.Fail( - RimMind.Application.Common.Threading.SynchronousTaskGate.AsyncNotSupportedMessage, - call.Id, - call.Name)) - .ToList(); - } - - return executionTask.GetAwaiter().GetResult(); - } - catch (Exception exception) - { - RimMindErrors.Warn( - $"[RimMind-Advisor] Tool execution failed for {_pawn.Name.ToStringShort}: {exception.Message}"); - return calls - .Select(call => ToolResult.Fail( - $"Tool execution failed: {exception.Message}", - call.Id, - call.Name)) - .ToList(); - } + // Per-call isolation: one failing call must not rewrite the outcome of calls whose side + // effects already ran (that would send false feedback to the LLM). + return AdvisorToolCallBatchRunner.ExecuteOnMainThread( + _toolExecutor, + calls, + $"NPC-{_pawn.thingIDNumber}", + traceId, + playerApproved, + message => RimMindErrors.Warn($"{message} (pawn={_pawn.Name.ToStringShort})")); } private static ClientStructuredToolCall StripPreApproval(ClientStructuredToolCall toolCall) diff --git a/Source/Advisor/AdvisorGameComponent.cs b/Source/Advisor/AdvisorGameComponent.cs index ef4eace..033aa52 100644 --- a/Source/Advisor/AdvisorGameComponent.cs +++ b/Source/Advisor/AdvisorGameComponent.cs @@ -88,14 +88,11 @@ private void EvaluateAllPawns(RimMindAdvisorSettings settings) } if (!shouldTrigger) continue; - float scale = RimMindAPI.Settings.ActivityFrequencyScale; - int ticksGame = Find.TickManager.TicksGame; - int dynamicCooldown = AdvisorCooldownCalculator.CalculateCooldownForPawn(pawn, settings); - float cdMult = scale > 0.01f ? (1.0f / scale) : 1.0f; - dynamicCooldown = UnityEngine.Mathf.RoundToInt(dynamicCooldown * UnityEngine.Mathf.Clamp(cdMult, 0.35f, 3.5f)); - int advisorCooldownLeft = dynamicCooldown - (ticksGame - comp.LastRequestTick); - if (advisorCooldownLeft > 0 || !comp.CanRequestAdvice()) continue; + // Single cooldown source: CanRequestAdvice uses AdvisorCooldownCalculator's + // effective (activity-scaled) cooldown, so no second check here. + if (!comp.CanRequestAdvice()) continue; + float scale = RimMindAPI.Settings.ActivityFrequencyScale; float effectiveChance = UnityEngine.Mathf.Clamp(settings.macroCheckChance * scale, 0.05f, 0.95f); if (Rand.Value > effectiveChance) continue; diff --git a/Source/Advisor/AdvisorProviderRegistrar.cs b/Source/Advisor/AdvisorProviderRegistrar.cs index e6b9d58..0bcd083 100644 --- a/Source/Advisor/AdvisorProviderRegistrar.cs +++ b/Source/Advisor/AdvisorProviderRegistrar.cs @@ -17,6 +17,10 @@ internal static class AdvisorProviderRegistrar internal static void RegisterAll() { + // Register the typed event before any key names it, so the trigger resolves + // to a real event (unknown names never fall back to the base bus event). + AdvisorHistoryChangedEvent.Register(); + RimMindAPI.Context.ContextKeys.Register(new ContextProviderDef( "advisor_history", ContextLayer.L4_History, 0.8f, async (ctx, ct) => @@ -74,7 +78,8 @@ internal static void RegisterAll() sb.AppendLine($"- {r.action}: {r.reason} → {resultLabel}"); } return sb.ToString().TrimEnd(); - }, RimMindOwnerConsts.AdvisorModId, stalenessTicks: 3000, invalidationTriggers: new[] { "AdvisorEvent" })); + }, RimMindOwnerConsts.AdvisorModId, stalenessTicks: 3000, + invalidationTriggers: new[] { AdvisorHistoryChangedEvent.EventName })); RimMindAPI.Context.ContextKeys.Register(new ContextProviderDef( "actions_list", ContextLayer.L3_State, 0.85f, @@ -85,7 +90,7 @@ internal static void RegisterAll() var pawn = RimMindPawnLookup.FindPawnByNumber(ctx.PawnId); var text = BuildToolListText(pawn); return string.IsNullOrEmpty(text) ? null : text; - }, RimMindOwnerConsts.AdvisorModId, stalenessTicks: 750, invalidationTriggers: new[] { "AdvisorEvent" })); + }, RimMindOwnerConsts.AdvisorModId, stalenessTicks: 750)); // follows the tool registry; refreshed by staleness RimMindAPI.Context.ContextKeys.Register(new ContextProviderDef( "advisor_task", ContextLayer.L0_Static, 0.95f, @@ -98,7 +103,7 @@ internal static void RegisterAll() .Select(key => (string)key.Translate()) .Where(t => !string.IsNullOrEmpty(t))); return instruction; - }, RimMindOwnerConsts.AdvisorModId, stalenessTicks: 0, invalidationTriggers: new[] { "AdvisorEvent" })); + }, RimMindOwnerConsts.AdvisorModId, stalenessTicks: 0)); RegisterPublicProviders(); } diff --git a/Source/Advisor/AdvisorRequestAugmentationFactory.cs b/Source/Advisor/AdvisorRequestAugmentationFactory.cs index c71cc2e..3d4c291 100644 --- a/Source/Advisor/AdvisorRequestAugmentationFactory.cs +++ b/Source/Advisor/AdvisorRequestAugmentationFactory.cs @@ -44,7 +44,7 @@ public static LlmRequestEnvelope Create( return LlmRequestEnvelopeBuilder .ForScenario(RimMindAPI.Context.ScenarioDecision) - .WithModId("RimMind.Advisor") + .WithModId(AdvisorRequestIds.RequestModId) .WithNpcId(npcId) .WithSchema(schema) .WithSystemAugmentations(augmentations) diff --git a/Source/Advisor/AdvisorRequestIds.cs b/Source/Advisor/AdvisorRequestIds.cs new file mode 100644 index 0000000..90686f1 --- /dev/null +++ b/Source/Advisor/AdvisorRequestIds.cs @@ -0,0 +1,15 @@ +namespace RimMind.Advisor.Advisor +{ + /// + /// Advisor-owned identity used by Core's request queue. + /// The same id must be passed to LlmRequestEnvelopeBuilder.WithModId, to + /// RimMindAPI.ClearModCooldown / GetModCooldownTicksLeft, and exposed by + /// AdvisorModCooldown: Core stores per-mod cooldowns under the envelope ModId, + /// so any other spelling silently targets a cooldown key that never exists. + /// + public static class AdvisorRequestIds + { + /// Envelope ModId and Core queue cooldown key for every Advisor request. + public const string RequestModId = "RimMind.Advisor"; + } +} diff --git a/Source/Advisor/AdvisorTaskDriver.cs b/Source/Advisor/AdvisorTaskDriver.cs index 203825d..36b63de 100644 --- a/Source/Advisor/AdvisorTaskDriver.cs +++ b/Source/Advisor/AdvisorTaskDriver.cs @@ -174,7 +174,7 @@ public void RequestToolFeedback( var envelope = LlmRequestEnvelopeBuilder .ForScenario(RimMindAPI.Context.ScenarioDecision) - .WithModId("RimMind.Advisor") + .WithModId(AdvisorRequestIds.RequestModId) .WithNpcId(npcId) .WithSchema(_feedbackSession.Schema) .WithMessages(messages) diff --git a/Source/Advisor/AdvisorToolCallExecutor.cs b/Source/Advisor/AdvisorToolCallExecutor.cs index c6cc3c4..99c780d 100644 --- a/Source/Advisor/AdvisorToolCallExecutor.cs +++ b/Source/Advisor/AdvisorToolCallExecutor.cs @@ -4,6 +4,7 @@ using System.Threading.Tasks; using RimMind.Application.Common.Interfaces.Tools; using RimMind.Application.Common.Models.Tools; +using RimMind.Application.Common.Threading; using RimMind.Domain.Llm; using RimMind.Presentation.Api; @@ -56,13 +57,141 @@ public async Task> ExecuteAsync( IsPlayerApproved = playerApproved }; + results.Add(await ExecuteIsolatedAsync(handler, args, call, ct)); + } + + return results; + } + + /// + /// Runs one handler so that a throwing or faulted handler yields its own failed result + /// instead of aborting the batch; earlier calls may already have mutated the game, so + /// their real outcomes must still reach the feedback batch. Mirrors Core's + /// ToolCallDispatchMiddleware isolation (cancellation still propagates). + /// + private static async Task ExecuteIsolatedAsync( + IToolHandler handler, + ToolCallArgs args, + StructuredToolCall call, + CancellationToken ct) + { + try + { var result = await handler.ExecuteAsync(args, ct); - results.Add(result.Match( + return result.Match( ok => ok with { ToolCallId = call.Id, ToolName = call.Name }, - err => ToolResult.Fail(err.Message, call.Id, call.Name))); + err => ToolResult.Fail(err.Message, call.Id, call.Name)); + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + throw; } + catch (Exception exception) + { + return ToolResult.Fail( + $"Tool threw {exception.GetType().Name}: {exception.Message}", + call.Id, + call.Name); + } + } + } + /// + /// Main-thread runner for an Advisor tool batch. Each call is dispatched and completed on its + /// own, so a throwing executor, a malformed result or an asynchronous handler only fails that + /// call; calls whose side effects already ran keep their real result in the feedback batch, and + /// later calls never resume on a thread-pool continuation. + /// + internal static class AdvisorToolCallBatchRunner + { + internal static List ExecuteOnMainThread( + IAdvisorToolCallExecutor executor, + IReadOnlyList calls, + string? npcId, + string? traceId, + bool playerApproved, + Action? warn = null) + { + if (executor == null) throw new ArgumentNullException(nameof(executor)); + var results = new List(calls.Count); + foreach (var call in calls) + results.Add(ExecuteOne(executor, call, npcId, traceId, playerApproved, warn)); return results; } + + private static ToolResult ExecuteOne( + IAdvisorToolCallExecutor executor, + StructuredToolCall call, + string? npcId, + string? traceId, + bool playerApproved, + Action? warn) + { + try + { + // Approval travels via ToolCallArgs.IsPlayerApproved (trusted channel), never via LLM JSON. + var executionTask = executor.ExecuteAsync( + new[] { call }, + npcId, + traceId, + CancellationToken.None, + playerApproved); + + if (!executionTask.IsCompleted) + { + SynchronousTaskGate.ObserveFault(executionTask); + warn?.Invoke( + "[RimMind-Advisor][ToolCall][MainThreadWait] " + + $"Tool '{call.Name}' did not complete synchronously on main thread " + + $"(trace={traceId ?? "unknown"}, npc={npcId ?? "none"}); failing fast to avoid main-thread freeze."); + var asyncFail = ToolResult.Fail(SynchronousTaskGate.AsyncNotSupportedMessage, call.Id, call.Name); + RimMindAPI.Tools.RecordToolCallTrace( + traceId, + call.Name ?? string.Empty, + call.Arguments, + false, + asyncFail.Content); + return asyncFail; + } + + var single = executionTask.GetAwaiter().GetResult(); + if (single == null || single.Count != 1 || single[0] == null) + { + var countFail = ToolResult.Fail( + $"Tool executor returned {single?.Count ?? 0} results for one call.", + call.Id, + call.Name); + RimMindAPI.Tools.RecordToolCallTrace( + traceId, + call.Name ?? string.Empty, + call.Arguments, + false, + countFail.Content); + return countFail; + } + + var toolResult = single[0]; + RimMindAPI.Tools.RecordToolCallTrace( + traceId, + toolResult.ToolName ?? call.Name ?? string.Empty, + call.Arguments, + !toolResult.IsError, + toolResult.Content); + return toolResult; + } + catch (Exception exception) + { + warn?.Invoke( + $"[RimMind-Advisor] Tool '{call.Name}' execution failed (npc={npcId ?? "none"}): {exception.Message}"); + var exFail = ToolResult.Fail($"Tool execution failed: {exception.Message}", call.Id, call.Name); + RimMindAPI.Tools.RecordToolCallTrace( + traceId, + call.Name ?? string.Empty, + call.Arguments, + false, + exFail.Content); + return exFail; + } + } } } diff --git a/Source/Advisor/AdvisorToolRiskResolver.cs b/Source/Advisor/AdvisorToolRiskResolver.cs index cc89e30..20b4b27 100644 --- a/Source/Advisor/AdvisorToolRiskResolver.cs +++ b/Source/Advisor/AdvisorToolRiskResolver.cs @@ -1,5 +1,7 @@ using System; using System.Collections.Generic; +using RimMind.Application.Common.Interfaces.Tools; +using RimMind.Application.Common.Models.Tools; using RimMind.Domain.Agent.Policies; using RimMind.Domain.Enums; using RimMind.Presentation.Api; @@ -9,29 +11,14 @@ namespace RimMind.Advisor.Advisor { internal static class AdvisorToolRiskResolver { - private static readonly Dictionary ToolAutonomyDomainMap - = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { "actions_place_blueprint", AutonomyDomain.BlueprintPlacement }, - { "actions_plan_room_project", AutonomyDomain.BlueprintPlacement }, - { "actions_designate_resources", AutonomyDomain.ResourceMarking }, - { "actions_manage_zone", AutonomyDomain.ResourceMarking }, - { "actions_add_worktable_bill", AutonomyDomain.WorktableBills }, - { "actions_manage_equipment", AutonomyDomain.EmergencySurvival }, - { "actions_emergency_flee", AutonomyDomain.EmergencySurvival }, - { "actions_stabilize_rest", AutonomyDomain.EmergencySurvival }, - { "actions_eat_and_recreation", AutonomyDomain.EmergencySurvival }, - { "actions_triage_patient", AutonomyDomain.EmergencySurvival }, - }; - - public static RiskLevel Resolve(string toolName) + public static RiskLevel Resolve(string? toolName, ToolCallArgs? args = null) { if (string.IsNullOrWhiteSpace(toolName)) { return RiskLevel.Low; } - if (TryParseToolName(toolName, out var mechanismId, out var operation)) + if (TryParseToolName(toolName!, out var mechanismId, out var operation)) { try { @@ -46,14 +33,37 @@ public static RiskLevel Resolve(string toolName) } } - if (TryGetAutonomyDomain(toolName, out var domain)) + try + { + var policy = RimMindAPI.Tools.ClassifyPolicy(toolName, args); + if (policy.HasValue) + { + if (policy.Value.RequiresApproval) + { + return RiskLevel.Critical; + } + if (policy.Value.Domain.HasValue) + { + return MapAutonomyDomainRisk(policy.Value.Domain.Value); + } + if (policy.Value.Risk != RiskLevel.Medium) + { + return policy.Value.Risk; + } + } + } + catch + { + } + + if (TryGetAutonomyDomain(toolName, args, out var domain)) { return MapAutonomyDomainRisk(domain); } try { - var manifest = RimMindAPI.Tools.FindById(toolName)?.Definition?.Manifest; + var manifest = RimMindAPI.Tools.FindById(toolName!)?.Definition?.Manifest; if (manifest != null && (manifest.RequiresApproval || manifest.RiskLevel != RiskLevel.Medium)) { if (manifest.RequiresApproval) @@ -70,31 +80,107 @@ public static RiskLevel Resolve(string toolName) return RiskLevel.Low; } - public static bool TryGetAutonomyDomain(string? toolName, out AutonomyDomain domain) + public static RiskLevel ResolveRiskLevel(string? toolName, ToolCallArgs? args = null) + => Resolve(toolName, args); + + public static AutonomyDomain? ResolveAutonomyDomain(string? toolName, ToolCallArgs? args = null) { if (string.IsNullOrWhiteSpace(toolName)) { - domain = default; - return false; + return null; } - // Tool IDs may arrive dotted ("actions.emergency_flee", registry form) or underscored - // ("actions_emergency_flee", the OpenAI function-name form). Normalize like ToolRegistry does. - return ToolAutonomyDomainMap.TryGetValue(NormalizeToolId(toolName!), out domain); + try + { + var policy = RimMindAPI.Tools.ClassifyPolicy(toolName!, args); + if (policy.HasValue) + { + if (policy.Value.Domain.HasValue) + { + return policy.Value.Domain.Value; + } + if (!string.IsNullOrWhiteSpace(policy.Value.DomainId) && + Enum.TryParse(policy.Value.DomainId, ignoreCase: true, out var parsedDomain)) + { + return parsedDomain; + } + } + } + catch + { + } + + try + { + var manifest = RimMindAPI.Tools.FindById(toolName!)?.Definition?.Manifest; + if (manifest != null) + { + if (manifest.DefaultDomain.HasValue) + { + return manifest.DefaultDomain.Value; + } + if (!string.IsNullOrWhiteSpace(manifest.AutonomyDomainId) && + Enum.TryParse(manifest.AutonomyDomainId, ignoreCase: true, out var manifestDomain)) + { + return manifestDomain; + } + } + } + catch + { + } + + if (TryParseToolName(toolName!, out var mechanismId, out var operation) && + operation != MechanismOperationType.Query && + operation != MechanismOperationType.List && + operation != MechanismOperationType.Watch) + { + if (mechanismId.StartsWith("pawn.", StringComparison.OrdinalIgnoreCase) || + string.Equals(mechanismId, "pawn", StringComparison.OrdinalIgnoreCase)) + { + return AutonomyDomain.EmergencySurvival; + } + if (mechanismId.StartsWith("map.", StringComparison.OrdinalIgnoreCase) || + string.Equals(mechanismId, "map", StringComparison.OrdinalIgnoreCase)) + { + return AutonomyDomain.ResourceMarking; + } + if (mechanismId.StartsWith("world.", StringComparison.OrdinalIgnoreCase) || + string.Equals(mechanismId, "world", StringComparison.OrdinalIgnoreCase)) + { + return AutonomyDomain.HighRiskOperations; + } + } + + return null; } - internal static string NormalizeToolId(string toolName) => toolName.Trim().Replace('.', '_'); + public static bool TryGetAutonomyDomain(string? toolName, out AutonomyDomain domain) + => TryGetAutonomyDomain(toolName, null, out domain); + + public static bool TryGetAutonomyDomain(string? toolName, ToolCallArgs? args, out AutonomyDomain domain) + { + var resolved = ResolveAutonomyDomain(toolName, args); + if (resolved.HasValue) + { + domain = resolved.Value; + return true; + } - internal static IEnumerable MappedToolIds => ToolAutonomyDomainMap.Keys; + domain = default; + return false; + } - public static bool RequiresAutonomyApproval(Pawn? pawn, string? toolName) + internal static string NormalizeToolId(string toolName) => toolName.Trim().Replace('.', '_'); + + public static bool RequiresAutonomyApproval(Pawn? pawn, string? toolName, ToolCallArgs? args = null) { if (string.IsNullOrWhiteSpace(toolName)) { return false; } - if (TryGetAutonomyDomain(toolName, out var domain)) + if (TryGetAutonomyDomain(toolName, args, out var domain)) { try { @@ -110,6 +196,12 @@ public static bool RequiresAutonomyApproval(Pawn? pawn, string? toolName) try { + var policy = RimMindAPI.Tools.ClassifyPolicy(toolName!, args); + if (policy.HasValue && policy.Value.RequiresApproval) + { + return true; + } + var manifest = RimMindAPI.Tools.FindById(toolName!)?.Definition?.Manifest; if (manifest != null && manifest.RequiresApproval) { @@ -123,14 +215,17 @@ public static bool RequiresAutonomyApproval(Pawn? pawn, string? toolName) return false; } - public static bool IsAutonomyForbidden(Pawn? pawn, string? toolName) + public static bool RequiresPlayerApproval(Pawn? pawn, string? toolName, ToolCallArgs? args = null) + => RequiresAutonomyApproval(pawn, toolName, args); + + public static bool IsAutonomyForbidden(Pawn? pawn, string? toolName, ToolCallArgs? args = null) { if (string.IsNullOrWhiteSpace(toolName)) { return false; } - if (TryGetAutonomyDomain(toolName, out var domain)) + if (TryGetAutonomyDomain(toolName, args, out var domain)) { try { @@ -145,7 +240,7 @@ public static bool IsAutonomyForbidden(Pawn? pawn, string? toolName) return false; } - public static bool ShouldSkipAction(string? toolName, Pawn? pawn = null) + public static bool ShouldSkipAction(string? toolName, Pawn? pawn = null, ToolCallArgs? args = null) { if (string.IsNullOrWhiteSpace(toolName)) { @@ -157,7 +252,7 @@ public static bool ShouldSkipAction(string? toolName, Pawn? pawn = null) return true; } - if (IsAutonomyForbidden(pawn, toolName)) + if (IsAutonomyForbidden(pawn, toolName, args)) { return true; } diff --git a/Source/Advisor/ApprovalManager.cs b/Source/Advisor/ApprovalManager.cs index 3d638df..0334b16 100644 --- a/Source/Advisor/ApprovalManager.cs +++ b/Source/Advisor/ApprovalManager.cs @@ -307,7 +307,17 @@ internal static string FormatHumanReadableToolName(string? rawName) return "Action"; var trimmed = rawName!.Trim(); - string[] prefixes = { "actions_", "pawn_", "rimmind_" }; + try + { + var displayName = RimMindAPI.Tools.FindById(trimmed)?.Definition?.Manifest?.DisplayName; + if (!string.IsNullOrWhiteSpace(displayName)) + return displayName!; + } + catch + { + } + + string[] prefixes = { "pawn_", "rimmind_" }; foreach (var prefix in prefixes) { if (trimmed.StartsWith(prefix, StringComparison.OrdinalIgnoreCase) && diff --git a/Source/Comps/CompAIAdvisor.cs b/Source/Comps/CompAIAdvisor.cs index 9beefd9..5003f8c 100644 --- a/Source/Comps/CompAIAdvisor.cs +++ b/Source/Comps/CompAIAdvisor.cs @@ -36,7 +36,10 @@ private AdvisorCycleCoordinator Cycle public AdvisorTaskDriver? TaskDriver => _cycle?.TaskDriver; public int AdvisorCooldownTicksLeft => _cycle != null ? _cycle.CooldownTicksLeft - : System.Math.Max(0, AdvisorCooldownCalculator.CalculateCooldownForPawn(Pawn, Settings) - ((Find.TickManager?.TicksGame ?? 0) - _lastRequestTick)); + : AdvisorCooldownCalculator.CooldownTicksLeft( + AdvisorCooldownCalculator.CalculateCooldownForPawn(Pawn, Settings), + Find.TickManager?.TicksGame ?? 0, + _lastRequestTick); public bool CanRequestAdvice() => IsEnabled && !HasPendingRequest && IsEligible() && AdvisorCooldownTicksLeft <= 0; diff --git a/Source/Data/AdvisorHistoryChangedEvent.cs b/Source/Data/AdvisorHistoryChangedEvent.cs new file mode 100644 index 0000000..6d27e62 --- /dev/null +++ b/Source/Data/AdvisorHistoryChangedEvent.cs @@ -0,0 +1,38 @@ +using System; +using RimMind.Domain.Events; +using RimMind.Domain.ValueObjects; +using RimMind.Presentation.Api; + +namespace RimMind.Advisor.Data +{ + /// + /// Published on the Core bus whenever a pawn's advisor request history changes. + /// The advisor_history context key names as its invalidation + /// trigger, so the next prompt sees the newest approval/rejection instead of a cached copy. + /// + public sealed class AdvisorHistoryChangedEvent : AgentBusEvent + { + /// Bus event name used by context-key invalidation triggers. + public const string EventName = "AdvisorEvent"; + + public AdvisorHistoryChangedEvent(int pawnId) + { + PawnId = pawnId; + NpcId = pawnId > 0 ? "NPC-" + pawnId : string.Empty; + } + + /// Registers with the Core bus. Call before registering context keys. + internal static void Register() + { + try { RimMindAPI.Bus.RegisterEventType(EventName); } + catch (Exception ex) { RimMindErrors.Warn($"[RimMind-Advisor] Event type registration failed: {ex.Message}"); } + } + + /// Publishes from any thread; never throws. + internal static void Publish(int pawnId) + { + try { RimMindAPI.Bus.PublishOnMainThread(new AdvisorHistoryChangedEvent(pawnId)); } + catch (Exception ex) { RimMindErrors.Warn($"[RimMind-Advisor] History change notification failed: {ex.Message}"); } + } + } +} diff --git a/Source/Data/AdvisorHistoryStore.cs b/Source/Data/AdvisorHistoryStore.cs index 10a1a15..6d26dbe 100644 --- a/Source/Data/AdvisorHistoryStore.cs +++ b/Source/Data/AdvisorHistoryStore.cs @@ -63,6 +63,7 @@ public void AddRecord(Pawn? pawn, AdvisorRequestRecord record) _globalLog.Add(record); if (_globalLog.Count > 200) _globalLog.RemoveRange(0, _globalLog.Count - 200); + AdvisorHistoryChangedEvent.Publish(pawn.thingIDNumber); } public void PruneInactivePawns() @@ -111,4 +112,26 @@ public override void ExposeData() _globalLog = globalLog ?? new List(); } } + +#if RIMMIND_ADVISOR_TESTS + internal sealed class AdvisorHistoryChangedEvent : RimMind.Domain.Events.AgentBusEvent + { + public const string EventName = "AdvisorEvent"; + + public AdvisorHistoryChangedEvent(int pawnId) + { + PawnId = pawnId; + NpcId = pawnId > 0 ? "NPC-" + pawnId : string.Empty; + } + + public static void Register() + { + RimMind.Application.Features.AgentBus.AgentBusImpl.RegisterKnownEventType(EventName, typeof(AdvisorHistoryChangedEvent)); + } + + public static void Publish(int pawnId) + { + } + } +#endif } diff --git a/Source/Debug/AdvisorBehaviorAutotestSuite.cs b/Source/Debug/AdvisorBehaviorAutotestSuite.cs index fb56cf2..15281a3 100644 --- a/Source/Debug/AdvisorBehaviorAutotestSuite.cs +++ b/Source/Debug/AdvisorBehaviorAutotestSuite.cs @@ -141,8 +141,8 @@ public override void RunSuite(IInGameBehaviorSuiteContext context) context.Assert(AdvisorConcurrencyTracker.ActiveCount == startCount, "AdvisorConcurrencyTracker decremented correctly"); // 6. Advisor Mod Cooldown - RimMindAPI.ClearModCooldown("Advisor"); - int cdLeft = RimMindAPI.GetModCooldownTicksLeft("Advisor"); + RimMindAPI.ClearModCooldown(AdvisorRequestIds.RequestModId); + int cdLeft = RimMindAPI.GetModCooldownTicksLeft(AdvisorRequestIds.RequestModId); context.Assert(cdLeft == 0, "Advisor Core-level cooldown cleared successfully"); } } diff --git a/Source/Debug/AdvisorDebugActions.cs b/Source/Debug/AdvisorDebugActions.cs index 8babab9..86c3347 100644 --- a/Source/Debug/AdvisorDebugActions.cs +++ b/Source/Debug/AdvisorDebugActions.cs @@ -51,7 +51,7 @@ private static void ShowAdvisorStateSelected() ? $"Cooling down ({advisorLeft} ticks ~ {advisorLeft / 2500f:F2} game hours)" : "Ready"; - int coreLeft = RimMindAPI.GetModCooldownTicksLeft("Advisor"); + int coreLeft = RimMindAPI.GetModCooldownTicksLeft(AdvisorRequestIds.RequestModId); string coreCooldown = coreLeft > 0 ? $"Cooling down ({coreLeft} ticks ~ {coreLeft / 2500f:F2} game hours)" : "Ready"; @@ -180,7 +180,7 @@ private static void ListAllAdvisorStates() if (comp == null) continue; int advisorLeft = comp.AdvisorCooldownTicksLeft; - int coreLeft = RimMindAPI.GetModCooldownTicksLeft("Advisor"); + int coreLeft = RimMindAPI.GetModCooldownTicksLeft(AdvisorRequestIds.RequestModId); string aState = advisorLeft > 0 ? $"AdvisorCD{advisorLeft}t" : "AdvisorReady"; string cState = coreLeft > 0 ? $"CoreCD{coreLeft}t" : "CoreReady"; @@ -195,7 +195,7 @@ private static void ListAllAdvisorStates() actionType = DebugActionType.Action)] private static void ClearAllCooldowns() { - RimMindAPI.ClearModCooldown("Advisor"); + RimMindAPI.ClearModCooldown(AdvisorRequestIds.RequestModId); Log.Message("[RimMind-Advisor] Advisor Core-layer cooldown cleared. To clear Advisor-layer cooldowns, use Force Request Advice on each colonist."); } diff --git a/Source/Extensions/AdvisorModCooldown.cs b/Source/Extensions/AdvisorModCooldown.cs index 559c098..2ab788e 100644 --- a/Source/Extensions/AdvisorModCooldown.cs +++ b/Source/Extensions/AdvisorModCooldown.cs @@ -1,3 +1,4 @@ +using RimMind.Advisor.Advisor; using RimMind.Advisor.Settings; using RimMind.Application.Common.Interfaces.Extension; @@ -7,8 +8,13 @@ internal sealed class AdvisorModCooldown : IModCooldown { private readonly RimMindAdvisorSettings _settings; public AdvisorModCooldown(RimMindAdvisorSettings settings) { _settings = settings; } - public string Id => "Advisor"; - public string OwnerModId => "RimMind.Advisor"; - public int CooldownTicks => _settings.requestCooldownTicks; + // Must equal the envelope ModId: Core resolves and stores queue cooldowns by that key. + public string Id => AdvisorRequestIds.RequestModId; + public string OwnerModId => AdvisorRequestIds.RequestModId; + // Same activity scaling as the per-pawn cooldown (AdvisorCooldownCalculator), so the + // Core queue gate does not silently re-impose the unscaled interval. + public int CooldownTicks => AdvisorCooldownCalculator.ApplyActivityFrequencyScale( + _settings.requestCooldownTicks, + RimMind.Presentation.Api.RimMindAPI.Settings.ActivityFrequencyScale); } } diff --git a/Tests/Contracts/AdvisorActionBoundaryContracts.cs b/Tests/Contracts/AdvisorActionBoundaryContracts.cs index bbd10b9..2d681f7 100644 --- a/Tests/Contracts/AdvisorActionBoundaryContracts.cs +++ b/Tests/Contracts/AdvisorActionBoundaryContracts.cs @@ -116,6 +116,34 @@ public void Approval_boundary_combines_risk_request_and_terminal_semantics() RimMindAPI.Autonomy.Policy = RimMind.Domain.Agent.Policies.AgentAutonomyPolicy.CreateDefault(); try { + RimMindAPI.Tools.Definitions.Add(new ToolDefinition + { + Id = "actions_place_blueprint", + Manifest = new ToolManifest + { + DefaultDomain = RimMind.Domain.Agent.Policies.AutonomyDomain.BlueprintPlacement, + RiskLevel = RiskLevel.Medium + } + }); + RimMindAPI.Tools.Definitions.Add(new ToolDefinition + { + Id = "actions_add_worktable_bill", + Manifest = new ToolManifest + { + DefaultDomain = RimMind.Domain.Agent.Policies.AutonomyDomain.WorktableBills, + RiskLevel = RiskLevel.Medium + } + }); + RimMindAPI.Tools.Definitions.Add(new ToolDefinition + { + Id = "actions_designate_resources", + Manifest = new ToolManifest + { + DefaultDomain = RimMind.Domain.Agent.Policies.AutonomyDomain.ResourceMarking, + RiskLevel = RiskLevel.Low + } + }); + Assert.Equal(RiskLevel.Medium, AdvisorToolRiskResolver.Resolve("actions_place_blueprint")); Assert.Equal(RiskLevel.Medium, AdvisorToolRiskResolver.Resolve("actions_add_worktable_bill")); Assert.Equal(RiskLevel.Low, AdvisorToolRiskResolver.Resolve("actions_designate_resources")); @@ -131,6 +159,7 @@ public void Approval_boundary_combines_risk_request_and_terminal_semantics() } finally { + RimMindAPI.Tools.Definitions.Clear(); RimMindAPI.Autonomy.Policy = RimMind.Domain.Agent.Policies.AgentAutonomyPolicy.CreateDefault(); } }), diff --git a/Tests/Contracts/AdvisorQueueAndToolIsolationContracts.cs b/Tests/Contracts/AdvisorQueueAndToolIsolationContracts.cs new file mode 100644 index 0000000..4d050de --- /dev/null +++ b/Tests/Contracts/AdvisorQueueAndToolIsolationContracts.cs @@ -0,0 +1,297 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text.RegularExpressions; +using System.Threading; +using System.Threading.Tasks; +using RimMind.Advisor.Advisor; +using RimMind.Advisor.Settings; +using RimMind.Application.Common.Interfaces.Tools; +using RimMind.Application.Common.Models.Tools; +using RimMind.Application.Common.Threading; +using RimMind.Domain.Llm; +using RimMind.Domain.ValueObjects; +using RimMind.Presentation.Api; +using RimMind.Testing; +using Xunit; + +namespace RimMind.Advisor.Tests.Contracts +{ + /// + /// Regression contracts for queue identity (B7), per-call tool isolation (B10) and the single + /// activity-scaled cooldown source (B11). + /// + [Collection("Advisor runtime")] + public sealed class AdvisorQueueAndToolIsolationContracts + { + [Fact] + public void Queue_cooldown_key_is_the_envelope_mod_id() + { + ContractCaseRunner.Run( + ("initial request envelope uses the module-owned request id", () => + { + var envelope = AdvisorRequestAugmentationFactory.Create("NPC-1", null, false, null, null); + Assert.Equal(AdvisorRequestIds.RequestModId, envelope.ModId); + }), + ("registered queue cooldown resolves by exact envelope ModId", () => + { + var envelope = AdvisorRequestAugmentationFactory.Create("NPC-1", null, false, null, null); + var cooldown = new AdvisorModCooldown(new RimMindAdvisorSettings()); + Assert.Equal(envelope.ModId, cooldown.Id); + Assert.Equal(envelope.ModId, cooldown.OwnerModId); + }), + ("source never clears or reads a cooldown with a literal key or diverging ModId", () => + { + var literalCooldownKey = new Regex(@"(ClearModCooldown|GetModCooldownTicksLeft)\(\s*"""); + var literalModId = new Regex(@"WithModId\(\s*""(?[^""]*)"""); + foreach (var file in Directory.EnumerateFiles(SourceRoot(), "*.cs", SearchOption.AllDirectories) + .Where(path => !path.Contains(Path.DirectorySeparatorChar + "obj" + Path.DirectorySeparatorChar))) + { + string text = File.ReadAllText(file); + Assert.False(literalCooldownKey.IsMatch(text), + $"{Path.GetFileName(file)} passes a literal cooldown key; use AdvisorRequestIds.RequestModId."); + foreach (Match match in literalModId.Matches(text)) + Assert.Equal(AdvisorRequestIds.RequestModId, match.Groups["id"].Value); + } + }), + ("queue cooldown follows the same activity scaling as the pawn cooldown", () => + { + var settings = new RimMindAdvisorSettings { requestCooldownTicks = 6000 }; + var cooldown = new AdvisorModCooldown(settings); + float previous = RimMindAPI.Settings.ActivityFrequencyScale; + try + { + RimMindAPI.Settings.ActivityFrequencyScale = 2f; + Assert.Equal(3000, cooldown.CooldownTicks); + RimMindAPI.Settings.ActivityFrequencyScale = 1f; + Assert.Equal(6000, cooldown.CooldownTicks); + } + finally + { + RimMindAPI.Settings.ActivityFrequencyScale = previous; + } + })); + } + + [Fact] + public Task Throwing_tool_call_fails_alone_and_later_calls_still_run() + { + return ContractCaseRunner.RunAsync( + ("executor isolates a synchronously throwing handler", async () => + { + var (executor, third) = CreateExecutor(new ThrowingHandler("tool.two", faulted: false)); + var results = await executor.ExecuteAsync(ThreeCalls(), "npc", "trace", CancellationToken.None); + + AssertOkFailOk(results); + Assert.Contains("InvalidOperationException", results[1].Content, StringComparison.Ordinal); + Assert.Equal(1, third.Calls); + }), + ("executor isolates a handler returning a faulted task", async () => + { + var (executor, third) = CreateExecutor(new ThrowingHandler("tool.two", faulted: true)); + var results = await executor.ExecuteAsync(ThreeCalls(), "npc", "trace", CancellationToken.None); + + AssertOkFailOk(results); + Assert.Equal(1, third.Calls); + }), + ("cancellation still propagates instead of becoming a tool result", async () => + { + using var cts = new CancellationTokenSource(); + cts.Cancel(); + var handler = new ThrowingHandler("tool.two", faulted: false, cancel: true); + var executor = new AdvisorToolCallExecutor(id => id == handler.Id ? handler : null); + await Assert.ThrowsAnyAsync(() => executor.ExecuteAsync( + new[] { new StructuredToolCall { Id = "c2", Name = "tool.two" } }, null, null, cts.Token)); + })); + } + + [Fact] + public void Main_thread_batch_runner_keeps_real_results_of_other_calls() + { + ContractCaseRunner.Run( + ("call 2 of 3 throwing in the executor fails only call 2", () => + { + var fake = new ScriptedExecutor(call => call.Name == "tool.two" + ? throw new InvalidOperationException("boom") + : Task.FromResult(new List { ToolResult.Ok("done", call.Id, call.Name) })); + var warnings = new List(); + + var results = AdvisorToolCallBatchRunner.ExecuteOnMainThread( + fake, ThreeCalls(), "NPC-1", "trace", playerApproved: true, warnings.Add); + + AssertOkFailOk(results); + Assert.Contains("boom", results[1].Content, StringComparison.Ordinal); + Assert.Equal(new[] { "tool.one", "tool.two", "tool.three" }, fake.Executed); + Assert.All(fake.PlayerApproved, Assert.True); + Assert.Single(warnings); + }), + ("an asynchronous call fails fast without failing completed neighbours", () => + { + var pending = new TaskCompletionSource>(); + var fake = new ScriptedExecutor(call => call.Name == "tool.two" + ? pending.Task + : Task.FromResult(new List { ToolResult.Ok("done", call.Id, call.Name) })); + + var results = AdvisorToolCallBatchRunner.ExecuteOnMainThread( + fake, ThreeCalls(), "NPC-1", null, playerApproved: false); + + AssertOkFailOk(results); + Assert.Equal(SynchronousTaskGate.AsyncNotSupportedMessage, results[1].Content); + Assert.Equal(3, fake.Executed.Count); + }), + ("a malformed executor result is reported for that call only", () => + { + var fake = new ScriptedExecutor(call => Task.FromResult(call.Name == "tool.two" + ? new List() + : new List { ToolResult.Ok("done", call.Id, call.Name) })); + + AssertOkFailOk(AdvisorToolCallBatchRunner.ExecuteOnMainThread( + fake, ThreeCalls(), null, null, playerApproved: false)); + })); + } + + [Fact] + public void Effective_cooldown_is_the_single_activity_scaled_source() + { + ContractCaseRunner.Run( + ("higher activity frequency shortens the effective cooldown", () => + { + Assert.Equal(15000, AdvisorCooldownCalculator.CalculateEffectiveCooldown(30000, false, false, false, 2f)); + Assert.Equal(30000, AdvisorCooldownCalculator.CalculateEffectiveCooldown(30000, false, false, false, 1f)); + Assert.Equal(60000, AdvisorCooldownCalculator.CalculateEffectiveCooldown(30000, false, false, false, 0.5f)); + }), + ("scaling is clamped and invalid scales are neutral", () => + { + Assert.Equal(10500, AdvisorCooldownCalculator.CalculateEffectiveCooldown(30000, false, false, false, 100f)); + Assert.Equal(105000, AdvisorCooldownCalculator.CalculateEffectiveCooldown(30000, false, false, false, 0.1f)); + Assert.Equal(30000, AdvisorCooldownCalculator.CalculateEffectiveCooldown(30000, false, false, false, 0f)); + }), + ("crisis safety floor still applies after scaling", () => + { + Assert.Equal( + AdvisorCooldownCalculator.MinCrisisCooldownTicks, + AdvisorCooldownCalculator.CalculateEffectiveCooldown(30000, false, true, false, 3f)); + }), + ("a pawn is ready once the scaled cooldown elapsed even if the base one has not", () => + { + int effective = AdvisorCooldownCalculator.CalculateEffectiveCooldown(30000, false, false, false, 2f); + Assert.Equal(0, AdvisorCooldownCalculator.CooldownTicksLeft(effective, nowTicks: 116000, lastRequestTick: 100000)); + int unscaled = AdvisorCooldownCalculator.CalculateEffectiveCooldown(30000, false, false, false, 1f); + Assert.Equal(14000, AdvisorCooldownCalculator.CooldownTicksLeft(unscaled, nowTicks: 116000, lastRequestTick: 100000)); + })); + } + + private static StructuredToolCall[] ThreeCalls() => new[] + { + new StructuredToolCall { Id = "c1", Name = "tool.one", Arguments = "{}" }, + new StructuredToolCall { Id = "c2", Name = "tool.two", Arguments = "{}" }, + new StructuredToolCall { Id = "c3", Name = "tool.three", Arguments = "{}" } + }; + + private static void AssertOkFailOk(IReadOnlyList results) + { + Assert.Equal(3, results.Count); + Assert.False(results[0].IsError); + Assert.True(results[1].IsError); + Assert.False(results[2].IsError); + Assert.Equal(new[] { "c1", "c2", "c3" }, results.Select(result => result.ToolCallId)); + Assert.Equal(new[] { "tool.one", "tool.two", "tool.three" }, results.Select(result => result.ToolName)); + } + + private static (AdvisorToolCallExecutor executor, CountingHandler third) CreateExecutor(IToolHandler second) + { + var first = new CountingHandler("tool.one"); + var third = new CountingHandler("tool.three"); + var handlers = new Dictionary + { + [first.Id] = first, + [second.Id] = second, + [third.Id] = third + }; + return (new AdvisorToolCallExecutor(id => handlers.TryGetValue(id, out var handler) ? handler : null), third); + } + + private static string SourceRoot() + { + var directory = new DirectoryInfo(AppContext.BaseDirectory); + while (directory != null && + !Directory.Exists(Path.Combine(directory.FullName, "RimMind-Advisor", "Source"))) + directory = directory.Parent; + + return Path.Combine( + directory?.FullName ?? throw new InvalidOperationException("Repository root not found."), + "RimMind-Advisor", + "Source"); + } + + private sealed class ScriptedExecutor : IAdvisorToolCallExecutor + { + private readonly Func>> _script; + public ScriptedExecutor(Func>> script) => _script = script; + public List Executed { get; } = new(); + public List PlayerApproved { get; } = new(); + + public Task> ExecuteAsync( + IReadOnlyList calls, + string? npcId, + string? traceId, + CancellationToken ct, + bool playerApproved = false) + { + var call = Assert.Single(calls); + Executed.Add(call.Name); + PlayerApproved.Add(playerApproved); + return _script(call); + } + } + + private sealed class CountingHandler : IToolHandler + { + public CountingHandler(string id) + { + Id = id; + Definition = new ToolDefinition { Id = id }; + } + + public string Id { get; } + public string OwnerModId => "RimMind.Advisor.Tests"; + public ToolDefinition Definition { get; } + public int Calls { get; private set; } + + public Task> ExecuteAsync(ToolCallArgs args, CancellationToken ct) + { + Calls++; + return Task.FromResult(Result.Ok(ToolResult.Ok("done"))); + } + } + + private sealed class ThrowingHandler : IToolHandler + { + private readonly bool _faulted; + private readonly bool _cancel; + + public ThrowingHandler(string id, bool faulted, bool cancel = false) + { + Id = id; + _faulted = faulted; + _cancel = cancel; + Definition = new ToolDefinition { Id = id }; + } + + public string Id { get; } + public string OwnerModId => "RimMind.Advisor.Tests"; + public ToolDefinition Definition { get; } + + public Task> ExecuteAsync(ToolCallArgs args, CancellationToken ct) + { + if (_cancel) + ct.ThrowIfCancellationRequested(); + var exception = new InvalidOperationException("handler exploded after partial side effects"); + if (_faulted) + return Task.FromException>(exception); + throw exception; + } + } + } +} diff --git a/Tests/Contracts/AdvisorToolDomainMapContracts.cs b/Tests/Contracts/AdvisorToolDomainMapContracts.cs index e3fff21..33c24be 100644 --- a/Tests/Contracts/AdvisorToolDomainMapContracts.cs +++ b/Tests/Contracts/AdvisorToolDomainMapContracts.cs @@ -2,45 +2,241 @@ using System.IO; using System.Linq; using System.Text.RegularExpressions; +using System.Threading; +using System.Threading.Tasks; +using Newtonsoft.Json.Linq; using RimMind.Advisor.Advisor; +using RimMind.Application.Common.Interfaces.Tools; +using RimMind.Application.Common.Models.Tools; +using RimMind.Domain.Agent.Policies; +using RimMind.Domain.Enums; +using RimMind.Domain.ValueObjects; +using RimMind.Presentation.Api; +using Verse; using Xunit; namespace RimMind.Advisor.Tests.Contracts { + [Collection("Advisor runtime")] public sealed class AdvisorToolDomainMapContracts { private static readonly Regex ToolIdPattern = new Regex("override string Id\\s*=>\\s*\"(?[^\"]+)\"", RegexOptions.Compiled); + private static readonly Regex DefaultDomainPattern = + new Regex("DefaultAutonomyDomain\\s*=>\\s*AutonomyDomain\\.(?[A-Za-z0-9_]+)", RegexOptions.Compiled); [Fact] - public void Every_mutating_Actions_composite_tool_id_resolves_an_autonomy_domain() + public void AdvisorToolRiskResolver_Contains_No_Hardcoded_Actions_ToolIds_Or_DomainMap() + { + var resolverPath = Path.Combine( + FindRepoRoot(), + "RimMind-Advisor", + "Source", + "Advisor", + "AdvisorToolRiskResolver.cs"); + Assert.True(File.Exists(resolverPath), $"Missing {resolverPath}"); + var source = File.ReadAllText(resolverPath); + + Assert.DoesNotContain("ToolAutonomyDomainMap", source, StringComparison.Ordinal); + Assert.DoesNotContain("actions_", source, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("actions.", source, StringComparison.OrdinalIgnoreCase); + Assert.Contains("ClassifyPolicy", source, StringComparison.Ordinal); + } + + [Fact] + public void Every_mutating_Actions_composite_tool_id_resolves_an_autonomy_domain_via_manifest() { - // Guards against the dotted-vs-underscored ID drift that silently disabled autonomy checks - // for actions.emergency_flee / stabilize_rest / eat_and_recreation / triage_patient. var actionsDir = Path.Combine(FindRepoRoot(), "RimMind-Actions", "Source", "Actions"); Assert.True(Directory.Exists(actionsDir), $"Actions source not found at {actionsDir}"); - var ids = Directory.GetFiles(actionsDir, "*CompositeTool.cs") - .Select(File.ReadAllText) - .Select(src => ToolIdPattern.Match(src)) - .Where(m => m.Success) - .Select(m => m.Groups["id"].Value) - .Where(id => id.IndexOf("query", StringComparison.OrdinalIgnoreCase) < 0) // read-only tools need no domain - .ToList(); - - Assert.NotEmpty(ids); - var unmapped = ids.Where(id => !AdvisorToolRiskResolver.TryGetAutonomyDomain(id, out _)).ToList(); - Assert.True(unmapped.Count == 0, "Unmapped Actions tool ids: " + string.Join(", ", unmapped)); + RimMindAPI.Tools.Definitions.Clear(); + RimMindAPI.Tools.Handlers.Clear(); + try + { + var files = Directory.GetFiles(actionsDir, "*CompositeTool.cs"); + var mutatingIds = new System.Collections.Generic.List(); + + foreach (var file in files) + { + var src = File.ReadAllText(file); + var idMatch = ToolIdPattern.Match(src); + if (!idMatch.Success) continue; + + var id = idMatch.Groups["id"].Value; + if (id.IndexOf("query", StringComparison.OrdinalIgnoreCase) >= 0) continue; + + var domainMatch = DefaultDomainPattern.Match(src); + Assert.True(domainMatch.Success, $"Composite tool '{id}' must declare DefaultAutonomyDomain in its manifest"); + var domain = (AutonomyDomain)Enum.Parse(typeof(AutonomyDomain), domainMatch.Groups["domain"].Value); + + RimMindAPI.Tools.Definitions.Add(new ToolDefinition + { + Id = id, + Manifest = new ToolManifest + { + OwnerModId = "RimMindActions", + DefaultDomain = domain, + AutonomyDomainId = domain.ToString() + } + }); + mutatingIds.Add(id); + } + + Assert.NotEmpty(mutatingIds); + var unmapped = mutatingIds + .Where(id => !AdvisorToolRiskResolver.TryGetAutonomyDomain(id, out _)) + .ToList(); + Assert.True(unmapped.Count == 0, "Unmapped Actions tool ids: " + string.Join(", ", unmapped)); + } + finally + { + RimMindAPI.Tools.Definitions.Clear(); + RimMindAPI.Tools.Handlers.Clear(); + } } [Theory] [InlineData("actions.emergency_flee")] [InlineData("actions_emergency_flee")] + [InlineData("emergency_flee")] [InlineData(" actions.triage_patient ")] - public void Dotted_and_underscored_ids_resolve_identically(string toolName) + public void Dotted_underscored_and_alias_ids_resolve_identically_from_manifest(string toolName) { - Assert.True(AdvisorToolRiskResolver.TryGetAutonomyDomain(toolName, out var domain)); - Assert.Equal(RimMind.Domain.Agent.Policies.AutonomyDomain.EmergencySurvival, domain); + RimMindAPI.Tools.Definitions.Clear(); + try + { + RimMindAPI.Tools.Definitions.Add(new ToolDefinition + { + Id = "actions.emergency_flee", + Manifest = new ToolManifest + { + DefaultDomain = AutonomyDomain.EmergencySurvival, + Aliases = new[] { "emergency_flee", "actions_emergency_flee" } + } + }); + RimMindAPI.Tools.Definitions.Add(new ToolDefinition + { + Id = "actions.triage_patient", + Manifest = new ToolManifest + { + DefaultDomain = AutonomyDomain.EmergencySurvival, + Aliases = new[] { "triage_patient", "actions_triage_patient" } + } + }); + + Assert.True(AdvisorToolRiskResolver.TryGetAutonomyDomain(toolName, out var domain)); + Assert.Equal(AutonomyDomain.EmergencySurvival, domain); + } + finally + { + RimMindAPI.Tools.Definitions.Clear(); + } + } + + [Fact] + public void Dynamic_IToolPolicyClassifier_escalates_survival_bed_and_meal_bill_fixing_B4() + { + var pawn = new Pawn { thingIDNumber = 7 }; + RimMindAPI.Autonomy.Policy = AgentAutonomyPolicy.CreateDefault(); + RimMindAPI.Tools.Handlers.Clear(); + RimMindAPI.Tools.Definitions.Clear(); + try + { + RimMindAPI.Tools.Handlers.Add(new DynamicBlueprintClassifierHandler()); + + // Static manifest lookup (no args) resolves to BlueprintPlacement + Assert.Equal( + AutonomyDomain.BlueprintPlacement, + AdvisorToolRiskResolver.ResolveAutonomyDomain("actions_place_blueprint")); + Assert.Equal( + RiskLevel.Medium, + AdvisorToolRiskResolver.Resolve("actions_place_blueprint")); + + // Survival Bed args escalate to EmergencySurvival (FullAutonomous by default -> no approval required!) + var bedArgs = new ToolCallArgs + { + ToolName = "actions_place_blueprint", + ArgumentsJson = "{\"building_def\":\"Bed\"}", + PawnId = pawn.thingIDNumber + }; + Assert.Equal( + AutonomyDomain.EmergencySurvival, + AdvisorToolRiskResolver.ResolveAutonomyDomain("actions_place_blueprint", bedArgs)); + Assert.Equal( + RiskLevel.Low, + AdvisorToolRiskResolver.Resolve("actions_place_blueprint", bedArgs)); + Assert.False( + AdvisorToolRiskResolver.RequiresAutonomyApproval(pawn, "actions_place_blueprint", bedArgs)); + + // Luxury statue args stay in BlueprintPlacement (RequireApproval by default -> requires approval!) + var statueArgs = new ToolCallArgs + { + ToolName = "actions_place_blueprint", + ArgumentsJson = "{\"building_def\":\"SculptureSmall\"}", + PawnId = pawn.thingIDNumber + }; + Assert.Equal( + AutonomyDomain.BlueprintPlacement, + AdvisorToolRiskResolver.ResolveAutonomyDomain("actions_place_blueprint", statueArgs)); + Assert.Equal( + RiskLevel.Medium, + AdvisorToolRiskResolver.Resolve("actions_place_blueprint", statueArgs)); + Assert.True( + AdvisorToolRiskResolver.RequiresAutonomyApproval(pawn, "actions_place_blueprint", statueArgs)); + } + finally + { + RimMindAPI.Tools.Handlers.Clear(); + RimMindAPI.Tools.Definitions.Clear(); + RimMindAPI.Autonomy.Policy = AgentAutonomyPolicy.CreateDefault(); + } + } + + private sealed class DynamicBlueprintClassifierHandler : IToolHandler, IToolPolicyClassifier + { + public string Id => "actions_place_blueprint"; + public string OwnerModId => "RimMindActions"; + + public ToolDefinition Definition { get; } = new ToolDefinition + { + Id = "actions_place_blueprint", + Manifest = new ToolManifest + { + OwnerModId = "RimMindActions", + DefaultDomain = AutonomyDomain.BlueprintPlacement, + RiskLevel = RiskLevel.Medium, + Aliases = new[] { "place_blueprint", "actions.place_blueprint" } + } + }; + + public ToolPolicyDecision Classify(ToolCallArgs args) + { + string? buildingDef = null; + if (!string.IsNullOrWhiteSpace(args?.ArgumentsJson)) + { + try + { + buildingDef = JObject.Parse(args!.ArgumentsJson)["building_def"]?.ToString(); + } + catch + { + } + } + + bool isSurvival = string.Equals(buildingDef, "Bed", StringComparison.OrdinalIgnoreCase) + || string.Equals(buildingDef, "Campfire", StringComparison.OrdinalIgnoreCase); + var domain = isSurvival ? AutonomyDomain.EmergencySurvival : AutonomyDomain.BlueprintPlacement; + return new ToolPolicyDecision + { + Domain = domain, + DomainId = domain.ToString(), + Risk = isSurvival ? RiskLevel.Low : RiskLevel.Medium, + RequiresApproval = false + }; + } + + public Task> ExecuteAsync(ToolCallArgs args, CancellationToken ct) + => Task.FromResult(Result.Ok(ToolResult.Ok("ok", args.ToolCallId, Id))); } private static string FindRepoRoot() diff --git a/Tests/RimMindAdvisor.Tests.csproj b/Tests/RimMindAdvisor.Tests.csproj index 2143c57..da0b035 100644 --- a/Tests/RimMindAdvisor.Tests.csproj +++ b/Tests/RimMindAdvisor.Tests.csproj @@ -27,6 +27,8 @@ + + diff --git a/Tests/VerseStubs.cs b/Tests/VerseStubs.cs index 98f25b9..2f43a0f 100644 --- a/Tests/VerseStubs.cs +++ b/Tests/VerseStubs.cs @@ -234,6 +234,7 @@ public static class Settings public static RimMind.Application.Common.Interfaces.Context.IContextBuilder? ContextEngine { get; set; } public static RimMind.Application.Common.Interfaces.Context.IContextBuilder? GetContextEngine() => ContextEngine; public static bool DebugLogging => false; + public static float ActivityFrequencyScale { get; set; } = 1f; } public static bool IsConfigured() => true; @@ -279,12 +280,96 @@ public StubToolHandler(RimMind.Application.Common.Models.Tools.ToolDefinition de public static class Tools { public static List Definitions { get; } = new(); + public static List Handlers { get; } = new(); + public static List<(string? RequestId, string ToolName, string? ArgumentsJson, bool Succeeded, string? ResultSummary)> RecordedTraces { get; } = new(); + public static IReadOnlyList GetAllDefinitions() => Definitions; + public static RimMind.Application.Common.Interfaces.Tools.IToolHandler? FindById(string id) { - var def = Definitions.Find(definition => definition.Id == id); + if (string.IsNullOrWhiteSpace(id)) return null; + var trimmed = id.Trim(); + var normalized = trimmed.Replace('.', '_'); + + foreach (var handler in Handlers) + { + if (MatchesToolIdOrAlias(handler.Definition, trimmed, normalized)) + return handler; + } + + var def = Definitions.Find(definition => MatchesToolIdOrAlias(definition, trimmed, normalized)); return def == null ? null : new StubToolHandler(def); } + + private static bool MatchesToolIdOrAlias( + RimMind.Application.Common.Models.Tools.ToolDefinition? def, + string trimmed, + string normalized) + { + if (def == null) return false; + if (string.Equals(def.Id, trimmed, StringComparison.OrdinalIgnoreCase) || + string.Equals(def.Id.Replace('.', '_'), normalized, StringComparison.OrdinalIgnoreCase)) + { + return true; + } + + var aliases = def.Manifest?.Aliases; + if (aliases != null) + { + for (int i = 0; i < aliases.Count; i++) + { + var alias = aliases[i]; + if (!string.IsNullOrWhiteSpace(alias) && + (string.Equals(alias.Trim(), trimmed, StringComparison.OrdinalIgnoreCase) || + string.Equals(alias.Trim().Replace('.', '_'), normalized, StringComparison.OrdinalIgnoreCase))) + { + return true; + } + } + } + + return false; + } + + public static RimMind.Application.Common.Interfaces.Tools.ToolPolicyDecision? ClassifyPolicy( + string? toolId, + RimMind.Application.Common.Models.Tools.ToolCallArgs? args = null) + { + if (string.IsNullOrWhiteSpace(toolId)) return null; + var handler = FindById(toolId!); + if (handler == null) return null; + + if (args != null && handler is RimMind.Application.Common.Interfaces.Tools.IToolPolicyClassifier classifier) + { + return classifier.Classify(args); + } + + var manifest = handler.Definition?.Manifest ?? RimMind.Application.Common.Models.Tools.ToolManifest.Default; + return new RimMind.Application.Common.Interfaces.Tools.ToolPolicyDecision + { + Domain = manifest.DefaultDomain, + DomainId = manifest.AutonomyDomainId ?? manifest.DefaultDomain?.ToString(), + Risk = manifest.RiskLevel, + RequiresApproval = manifest.RequiresApproval + }; + } + + public static RimMind.Application.Common.Interfaces.Tools.ToolPolicyDecision? Preflight( + string? toolId, + RimMind.Application.Common.Models.Tools.ToolCallArgs? args = null) + => ClassifyPolicy(toolId, args); + + public static void RecordToolCallTrace( + string? requestId, + string toolName, + string? argumentsJson, + bool succeeded, + string? resultSummary) + { + if (string.IsNullOrWhiteSpace(requestId) || string.IsNullOrWhiteSpace(toolName)) + return; + RecordedTraces.Add((requestId, toolName, argumentsJson, succeeded, resultSummary)); + } } public static class Request