Fix benchmark maintained aura selection (#18)

docs/ISSUE_18_MAINTENANCE_CAST_SELECTION.md

@@ -0,0 +1,22 @@
+# Issue 18: maintenance cast selection
+
+The benchmark controller scores a generic candidate list alongside the shared
+rotation. A maintained aura rejected by the shared selector must also be
+removed from that fallback list, or the next benchmark decision can cast it
+again.
+
+Current, unexpired auras are passed to the selector with their actual target
+and aura ID. The selector suppresses the same applied aura only on the target
+the candidate would use. An aura on another actor does not block the cast, and
+a different aura in the same exclusive group remains eligible to replace it.
+Expired and dispelled auras are absent from the active snapshot. Periodic
+effects retain their source, target, and remaining-duration checks. An attack
+or heal that also applies an aura remains eligible for its primary effect.
+
+Focused regressions cover Protection Paladin L60 Greater Blessing of Sanctuary,
+RoM Scout L30 Detection, target-scoped aura selection, and exclusive-group
+replacement. Run them with:
+
+```sh
+./node_modules/.bin/vitest run src/game/benchmark/benchmarkController.maintenance.test.ts src/game/combatRotation.test.ts --reporter=dot
+```

src/game/benchmark/benchmarkController.maintenance.test.ts

@@ -0,0 +1,72 @@
+import { beforeEach, describe, expect, it } from "vitest";
+import { abilityAtLevel, abilitiesForClass } from "../abilityCatalog";
+import { PLAYER_AGGRO_ID } from "../aggro";
+import { useCombatStore } from "../combatStore";
+import { clearMobRuntimeRegistry } from "../mobRuntimeRegistry";
+import { clearPartyRuntimeRegistry } from "../partyRuntimeRegistry";
+import { BenchmarkController } from "./benchmarkController";
+import { BENCHMARK_SCENARIOS } from "./benchmarkScenarios";
+
+const scenario = { ...BENCHMARK_SCENARIOS[0], hostileTargets: 1 };
+
+describe("benchmark maintenance aura fallback", () => {
+  beforeEach(() => {
+    clearMobRuntimeRegistry();
+    clearPartyRuntimeRegistry();
+  });
+
+  it("does not re-admit an active self buff after shared rotation rejects it", () => {
+    useCombatStore.getState().initializeCharacter({ classId: "paladin", role: "tank", level: 60 });
+    const sanctuary = abilityAtLevel(
+      abilitiesForClass("paladin").find((ability) => ability.name === "Greater Blessing of Sanctuary")!,
+      60,
+    );
+    useCombatStore.setState({ abilities: [sanctuary] });
+    expect(useCombatStore.getState().castAbility(sanctuary.id, { now: 100 }).ok).toBe(true);
+    expect(useCombatStore.getState().auras.some((aura) => aura.targetId === PLAYER_AGGRO_ID
+      && aura.definition.id === sanctuary.effects.find((effect) => effect.kind === "apply-aura")?.aura.id)).toBe(true);
+
+    const decision = new BenchmarkController().act(2_000, "tank", scenario, false);
+
+    expect(decision.cast).toBe(false);
+    expect(useCombatStore.getState().auras.filter((aura) => aura.definition.id
+      === sanctuary.effects.find((effect) => effect.kind === "apply-aura")?.aura.id)).toHaveLength(1);
+  });
+
+  it("allows the same maintenance cast after its aura expires or is removed", () => {
+    useCombatStore.getState().initializeCharacter({ classId: "paladin", role: "tank", level: 60 });
+    const sanctuary = abilityAtLevel(
+      abilitiesForClass("paladin").find((ability) => ability.name === "Greater Blessing of Sanctuary")!,
+      60,
+    );
+    const auraId = sanctuary.effects.find((effect) => effect.kind === "apply-aura")!.aura.id;
+    useCombatStore.setState({ abilities: [sanctuary] });
+    expect(useCombatStore.getState().castAbility(sanctuary.id, { now: 100 }).ok).toBe(true);
+    useCombatStore.setState((state) => ({
+      auras: state.auras.map((aura) => aura.definition.id === auraId ? { ...aura, expiresAt: 1_999 } : aura),
+    }));
+
+    expect(new BenchmarkController().act(2_000, "tank", scenario, false).cast).toBe(true);
+
+    useCombatStore.setState({ auras: [] });
+    expect(new BenchmarkController().act(4_000, "tank", scenario, false).cast).toBe(true);
+  });
+
+  it("does not re-admit an active RoM self debuff through generic candidate scoring", () => {
+    useCombatStore.getState().initializeCharacter({ classId: "rom-scout", statRuleset: "rom", role: "damage", level: 30 });
+    const detection = abilityAtLevel(
+      abilitiesForClass("rom-scout").find((ability) => ability.id === "rom-scout-primary-8")!,
+      30,
+    );
+    useCombatStore.setState({ abilities: [detection] });
+    expect(useCombatStore.getState().castAbility(detection.id, { now: 100 }).ok).toBe(true);
+    expect(useCombatStore.getState().auras.some((aura) => aura.targetId === PLAYER_AGGRO_ID
+      && aura.definition.id === detection.effects.find((effect) => effect.kind === "apply-aura")?.aura.id)).toBe(true);
+
+    const decision = new BenchmarkController().act(2_000, "damage", scenario, false);
+
+    expect(decision.cast).toBe(false);
+    expect(useCombatStore.getState().auras.filter((aura) => aura.definition.id
+      === detection.effects.find((effect) => effect.kind === "apply-aura")?.aura.id)).toHaveLength(1);
+  });
+});

src/game/benchmark/benchmarkController.ts

@@ -25,6 +25,7 @@ import type { BenchmarkScenarioDefinition } from "./benchmarkTypes";
 import { nativeRotationAbilityAllowed, nativeRotationProfileFor } from "../romFiveRotation";
 import {
   COMBAT_ROTATION_VERSION,
+  abilityAuraMaintainedOnTarget,
   evaluateCombatRotation,
   nativeEarlyPriestHealReserveBlocksAbility,
   type RotationContext,
@@ -52,6 +53,15 @@ interface ScoredAbility {
   readonly friendlyTarget: FriendlyTarget | null;
 }
 
+function scoredAbilityAuraTargetId(candidate: ScoredAbility, combat: CombatState): string | null {
+  const { ability, friendlyTarget } = candidate;
+  if (ability.target === "friendly" || (ability.target === "any" && friendlyTarget)) {
+    return friendlyTarget?.id ?? combat.selectedFriendlyActorId ?? PLAYER_AGGRO_ID;
+  }
+  if (ability.target === "hostile" || ability.target === "any") return combat.selectedTargetId;
+  return PLAYER_AGGRO_ID;
+}
+
 const TRANSIENT_FAILURES = new Set<CastFailureReason>([
   "already-casting",
   "global-cooldown",
@@ -287,13 +297,10 @@ export class BenchmarkController {
       maintainedAbilityIds: new Set([
         ...combat.effects.filter((effect) => effect.sourceActorId === PLAYER_AGGRO_ID && effect.remainingTicks > 0)
           .map((effect) => effect.sourceAbilityId),
-        ...combat.auras.filter((aura) => aura.sourceId === PLAYER_AGGRO_ID
-          && (aura.expiresAt === null || aura.expiresAt > now))
-          .flatMap((aura) => availableAbilities
-            .filter((ability) => ability.effects.some((effect) => effect.kind === "apply-aura"
-              && effect.aura.id === aura.definition.id))
-            .map((ability) => ability.id)),
       ]),
+      maintainedAuras: combat.auras
+        .filter((aura) => aura.expiresAt === null || aura.expiresAt > now)
+        .map((aura) => ({ auraId: aura.definition.id, targetId: aura.targetId })),
       maintainedPeriodicEffects: combat.effects.map((effect) => ({
         abilityId: effect.sourceAbilityId,
         sourceActorId: effect.sourceActorId,
@@ -315,9 +322,11 @@ export class BenchmarkController {
     };
     const sharedDecision = evaluateCombatRotation(rotationContext);
     for (let index = candidates.length - 1; index >= 0; index -= 1) {
-      const ability = candidates[index].ability;
+      const candidate = candidates[index];
+      const ability = candidate.ability;
       if (nativeEarlyPriestHealReserveBlocksAbility(ability, rotationContext)
-        || (nativeRotationProfileFor(rotationContext) && !nativeRotationAbilityAllowed(ability, rotationContext))) {
+        || (nativeRotationProfileFor(rotationContext) && !nativeRotationAbilityAllowed(ability, rotationContext))
+        || abilityAuraMaintainedOnTarget(ability, rotationContext, scoredAbilityAuraTargetId(candidate, combat))) {
         candidates.splice(index, 1);
       }
     }

src/game/combatRotation.test.ts

@@ -11,6 +11,7 @@ import {
 import {
   abilityIsPrecombatPreparation,
   abilityHasRotationSemantics,
+  abilityAuraMaintainedOnTarget,
   CLASSIC_ROTATION_PROFILES,
   COMBAT_ROTATION_VERSION,
   evaluateCombatRotation,
@@ -764,6 +765,52 @@ describe("shared combat rotations", () => {
       .toBe(lightningBolt.id);
   });
 
+  it("matches maintained auras by recipient and exact effect while allowing a group override", () => {
+    const auraAbility = (id: string, auraId: string, recipient: "caster" | "ability-target" = "caster") => semanticAbility(id, [{
+      kind: "apply-aura",
+      aura: {
+        id: auraId,
+        name: id,
+        disposition: "buff",
+        durationMs: 300_000,
+        exclusiveGroup: "test:stance",
+        modifiers: [{ kind: "stat", stat: "armor", operation: "flat", value: 10 }],
+      },
+      recipient,
+      sourceEffectType: 6,
+      sourceAuraType: 22,
+      miscValue: 0,
+      triggerSpellId: 0,
+    }], { target: "self" });
+    const stance = auraAbility("stance", "stance-aura");
+    const replacementStance = auraAbility("replacement-stance", "replacement-aura");
+    const allyAura = {
+      ...auraAbility("ally-aura", "ally-aura-id", "ability-target"),
+      target: "friendly" as const,
+    };
+    const activeOnSelf = context({
+      selfId: "player",
+      maintainedAuras: [{ auraId: "stance-aura", targetId: "player" }],
+    });
+
+    expect(abilityAuraMaintainedOnTarget(stance, activeOnSelf, "player")).toBe(true);
+    expect(abilityAuraMaintainedOnTarget(allyAura, activeOnSelf, "ally")).toBe(false);
+    expect(abilityAuraMaintainedOnTarget(stance, {
+      ...activeOnSelf,
+      maintainedAuras: [{ auraId: "stance-aura", targetId: "ally" }],
+    }, "player")).toBe(false);
+    expect(abilityAuraMaintainedOnTarget(allyAura, {
+      ...activeOnSelf,
+      maintainedAuras: [{ auraId: "ally-aura-id", targetId: "player" }],
+    }, "player")).toBe(true);
+    expect(abilityAuraMaintainedOnTarget(replacementStance, activeOnSelf, "player")).toBe(false);
+    expect(abilityAuraMaintainedOnTarget({
+      ...stance,
+      effects: [...stance.effects, { kind: "damage", coefficient: 1 }],
+    }, activeOnSelf, "player")).toBe(false);
+    expect(abilityAuraMaintainedOnTarget(stance, { ...activeOnSelf, maintainedAuras: [] }, "player")).toBe(false);
+  });
+
   it("maintains Earth Shield on a valid full-health ally", () => {
     const earthShield = ability("shaman", "Earth Shield");
     const shaman = context({

src/game/combatRotation.ts

@@ -94,6 +94,11 @@ export interface RotationContext {
     readonly targetId: string;
     readonly remainingMs: number;
   }[];
+  /** Active applied auras with their actual recipient, filtered for expiry. */
+  readonly maintainedAuras?: readonly {
+    readonly auraId: string;
+    readonly targetId: string;
+  }[];
   readonly activeAuraExclusiveGroups?: ReadonlySet<string>;
   readonly activeSummonAbilityIds?: ReadonlySet<string>;
   readonly hasActivePet?: boolean;
@@ -378,6 +383,36 @@ function targetForAbility(
   return null;
 }
 
+/** Whether this applied-aura action is already active on the recipient it would use. */
+export function abilityAuraMaintainedOnTarget(
+  ability: AbilityDefinition,
+  context: RotationContext,
+  targetId: string | null,
+): boolean {
+  // An aura can be a side effect of an attack. Keep those casts available for
+  // their damage or healing; this guard only handles maintenance actions.
+  if (ability.effects.some((effect) => effect.kind === "damage" || effect.kind === "dot"
+    || effect.kind === "heal" || effect.kind === "hot" || effect.kind === "shield"
+    || effect.kind === "finisher-damage" || effect.kind === "execute")) return false;
+  const auraEffects = ability.effects.filter((effect) => effect.kind === "apply-aura");
+  if (auraEffects.length === 0) return false;
+  const maintainedAuras = context.maintainedAuras;
+  if (maintainedAuras !== undefined) {
+    return auraEffects.some((effect) => {
+      const recipientId = effect.recipient === "caster"
+        ? context.selfId ?? null
+        : effect.recipient === "pet" ? null : targetId;
+      return recipientId !== null && maintainedAuras.some((aura) => (
+        aura.targetId === recipientId && aura.auraId === effect.aura.id
+      ));
+    });
+  }
+  // Older selector callers only expose caster-owned exclusive groups. Keep
+  // their conservative behavior until they provide recipient-scoped auras.
+  return auraEffects.some((effect) => Boolean(effect.aura.exclusiveGroup
+    && context.activeAuraExclusiveGroups?.has(effect.aura.exclusiveGroup)));
+}
+
 interface ScoredRotationAbility {
   readonly ability: AbilityDefinition;
   readonly score: number;
@@ -410,13 +445,20 @@ function scoreAbility(
     && ability.effects.some((effect) => effect.kind === "dot" || effect.kind === "hot");
   const coaPeriodic = context.maintainedPeriodicEffects !== undefined && Boolean(context.selfId)
     && usesCoaTargetedPeriodicMaintenance(ability, context);
+  const maintenanceTargetId = targetForAbility(ability, context, lowest)
+    ?? (ability.target === "self" ? context.selfId ?? null : null);
   const periodicMaintained = coaPeriodic
-    ? coaPeriodicActionMaintainedOnTarget(ability, context, targetForAbility(ability, context, lowest))
-    : !nativePeriodic && (context.maintainedAbilityIds?.has(ability.id) ?? false);
+    ? coaPeriodicActionMaintainedOnTarget(ability, context, maintenanceTargetId)
+    : !nativePeriodic && (context.maintainedPeriodicEffects !== undefined
+      ? context.maintainedPeriodicEffects.some((effect) => (
+        effect.abilityId === ability.id
+        && effect.sourceActorId === context.selfId
+        && effect.targetId === maintenanceTargetId
+        && effect.remainingMs > 0
+      ))
+      : (context.maintainedAbilityIds?.has(ability.id) ?? false));
   const maintained = periodicMaintained
-    || ability.effects.some((effect) => effect.kind === "apply-aura"
-      && Boolean(effect.aura.exclusiveGroup
-        && context.activeAuraExclusiveGroups?.has(effect.aura.exclusiveGroup)));
+    || abilityAuraMaintainedOnTarget(ability, context, maintenanceTargetId);
   const activeSummon = context.activeSummonAbilityIds?.has(ability.id) ?? false;
   const petSummon = isPetSummon(ability);
   const offensiveSummon = isOffensiveSummon(ability);