feat(core): training replaces, and the rare card boosts all six

Corrects two decisions the previous revision got wrong, both now settled by
the published FIFA 17 training guide -- the same source class and publisher
this project already accepted for the contract matrix, and which
cross-validates 6/6 against fcc_trainingcards on rows we had misclassified.

"You can only boost one attribute or all six... When you apply a new training
card to a player, he loses the improved attributes of previous training
cards. It does not accumulate, it replaces."

So a second card REPLACES rather than being refused, and two slots must never
be boosted at once -- our old composite key permitted exactly that, and
staging demonstrated it by holding a slot-4 and a slot-1 effect together.

Migration 0030 reshapes the table to one row per instance keyed on
owned_card_id alone, with attribute_index nullable for the rare all-six card,
and carries forward the most recent effect where several existed -- the
replaces rule applied retroactively. 0029 is left intact rather than
rewritten: it is already applied to supervised staging, where migration
history matters.

Apply is now delete-then-insert inside the one transaction, so the old effect
cannot survive a failed insert and the two cannot coexist. The outcome
records what it displaced instead of overwriting history silently.

The previous refusal was OUR policy standing in for an unknown, and it was
never evidence about FIFA 17. It is retired now that the behaviour is
recovered, not because the 409 was inconvenient.

src/services/instance_effect.rs

@@ -44,20 +44,29 @@ pub enum InstanceEffect {
         cap: i64,
         default_when_unset: i64,
     },
-    /// Attach an attribute training effect to the target.
+    /// Attach an attribute training effect to the target, REPLACING any the
+    /// instance already carries.
     ///
     /// `attribute_index` is a slot in Core's own six-attribute card model, in
-    /// `CardDefinition` declaration order (0 pace .. 5 physical). Naming the
-    /// slot rather than the game's attribute is what keeps this game-neutral:
-    /// that FIFA 17's "GK speed" is slot 4 is the adapter's reversed knowledge,
-    /// and it stays there.
+    /// `CardDefinition` declaration order (0 pace .. 5 physical), or `None` for
+    /// an effect that boosts ALL SIX slots. Naming the slot rather than the
+    /// game's attribute is what keeps this game-neutral: that FIFA 17's "GK
+    /// speed" is slot 4 is the adapter's reversed knowledge, and it stays there.
     ///
-    /// `max_amount` is the caller's authored ceiling for its own family (FIFA 17
-    /// authors 5/10/15, so 15). Core cannot know it, but it can refuse anything
-    /// above the number the caller itself declares, which is what stops a host
-    /// from describing a "+99 pace" that no card could grant.
+    /// REPLACEMENT, NOT ACCUMULATION, and at most one effect per instance. Both
+    /// halves are the caller's game rule, but they are enforced here because the
+    /// storage shape is Core's: FIFA 17's own documentation states "you can only
+    /// boost one attribute or all six" and "when you apply a new training card
+    /// to a player, he loses the improved attributes of previous training cards.
+    /// It does not accumulate, it replaces."
+    ///
+    /// `max_amount` is the caller's authored ceiling for the specific family
+    /// (FIFA 17: 15 single-attribute, 10 for the rare all-six card). Core cannot
+    /// know it, but it can refuse anything above the number the caller itself
+    /// declares, which is what stops a host describing a "+99 pace" no card
+    /// could grant.
     ApplyTraining {
-        attribute_index: i64,
+        attribute_index: Option<i64>,
         amount: i64,
         max_amount: i64,
     },
@@ -192,16 +201,18 @@ const ATTRIBUTE_SLOTS: i64 = 6;
 async fn apply_training(
     tx: &mut SqliteConnection,
     ctx: &ConsumeContext,
-    attribute_index: i64,
+    attribute_index: Option<i64>,
     amount: i64,
     max_amount: i64,
 ) -> AppResult<Value> {
     let target = require_target(ctx, "apply_training")?;
 
-    if !(0..ATTRIBUTE_SLOTS).contains(&attribute_index) {
-        return Err(AppError::BadRequest(format!(
-            "apply_training attribute_index must be 0..{ATTRIBUTE_SLOTS}, got {attribute_index}"
-        )));
+    if let Some(slot) = attribute_index {
+        if !(0..ATTRIBUTE_SLOTS).contains(&slot) {
+            return Err(AppError::BadRequest(format!(
+                "apply_training attribute_index must be 0..{ATTRIBUTE_SLOTS} or absent, got {slot}"
+            )));
+        }
     }
     if amount < 1 {
         return Err(AppError::BadRequest(format!(
@@ -223,7 +234,7 @@ async fn apply_training(
     }
     // Same reasoning as contracts: a loan is borrowed for a fixed run of
     // matches, so durably improving it would outlive the thing it is attached
-    // to. Conservative and consistent rather than reversed — no FIFA 17 source
+    // to. Conservative and consistent rather than reversed -- no FIFA 17 source
     // speaks to training a loan item.
     if target.is_loan {
         return Err(AppError::BadRequest(format!(
@@ -232,18 +243,28 @@ async fn apply_training(
         )));
     }
 
-    // The INSERT is the check. `(owned_card_id, attribute_index)` is the primary
-    // key, so a second training on a slot that already carries one collides here
-    // and the whole transaction rolls back — the source is NOT consumed. That is
-    // deliberate: replacement-vs-stacking is UNKNOWN (see migration 0029), and a
-    // refusal is the only answer that neither invents a rule nor silently eats a
-    // card. Detected as a constraint violation rather than a SELECT-then-INSERT
-    // so it holds against a concurrent writer instead of racing it.
-    let inserted = sqlx::query(
+    // REPLACE. One instance carries at most one training effect, and a new card
+    // supersedes whatever was there -- including an effect on a DIFFERENT slot,
+    // because FIFA 17 allows "one attribute or all six" and never a mixture.
+    // Delete-then-insert inside the caller's transaction, so the old effect can
+    // never survive a failed insert and the two can never coexist.
+    let previous = sqlx::query_as::<_, (Option<i64>, i64, String)>(
+        "SELECT attribute_index, amount, source_card_id FROM owned_card_training \
+         WHERE owned_card_id = ?",
+    )
+    .bind(&target.id)
+    .fetch_optional(&mut *tx)
+    .await?;
+
+    sqlx::query("DELETE FROM owned_card_training WHERE owned_card_id = ?")
+        .bind(&target.id)
+        .execute(&mut *tx)
+        .await?;
+
+    sqlx::query(
         "INSERT INTO owned_card_training \
            (owned_card_id, attribute_index, amount, source_card_id, applied_at) \
-         VALUES (?, ?, ?, ?, ?) \
-         ON CONFLICT(owned_card_id, attribute_index) DO NOTHING",
+         VALUES (?, ?, ?, ?, ?)",
     )
     .bind(&target.id)
     .bind(attribute_index)
@@ -251,17 +272,7 @@ async fn apply_training(
     .bind(&ctx.source.card_id)
     .bind(Utc::now().to_rfc3339())
     .execute(&mut *tx)
-    .await?
-    .rows_affected();
-
-    if inserted != 1 {
-        return Err(AppError::Conflict(format!(
-            "target item '{}' already carries training on attribute slot {attribute_index}; \
-             FIFA 17 replacement/stacking behaviour is unproven, so this is refused rather \
-             than guessed",
-            target.id
-        )));
-    }
+    .await?;
 
     Ok(json!({
         "kind": "apply_training",
@@ -270,12 +281,19 @@ async fn apply_training(
         // Named `granted` as well as `amount` so every effect's recorded outcome
         // answers "what did this card award" under one key, whatever the family.
         "granted": amount,
-        // `before`/`after` describe the TRAINING held on this slot, not the
+        // `before`/`after` describe the training this instance holds, not the
         // attribute's value: Core stores effects, and the attribute total is a
         // projection over a definition Core does not consult here. `before` is
-        // always 0 because a slot that already carried training refused above.
-        "before": 0,
+        // the magnitude of the effect this one replaced, 0 when there was none.
+        "before": previous.as_ref().map(|(_, a, _)| *a).unwrap_or(0),
         "after": amount,
+        // What was displaced, so the outcome records the replacement rather than
+        // silently overwriting history.
+        "replaced": previous.map(|(slot, amt, src)| json!({
+            "attribute_index": slot,
+            "amount": amt,
+            "source_card_id": src,
+        })),
     }))
 }
 
@@ -369,16 +387,25 @@ mod tests {
 
     fn train(attribute_index: i64, amount: i64) -> InstanceEffect {
         InstanceEffect::ApplyTraining {
-            attribute_index,
+            attribute_index: Some(attribute_index),
             amount,
             max_amount: 15,
         }
     }
 
-    async fn training_of(pool: &db::Pool, id: &str) -> Vec<(i64, i64, String)> {
-        sqlx::query_as::<_, (i64, i64, String)>(
+    /// The rare card: every slot, ceiling 10.
+    fn train_all(amount: i64) -> InstanceEffect {
+        InstanceEffect::ApplyTraining {
+            attribute_index: None,
+            amount,
+            max_amount: 10,
+        }
+    }
+
+    async fn training_of(pool: &db::Pool, id: &str) -> Vec<(Option<i64>, i64, String)> {
+        sqlx::query_as::<_, (Option<i64>, i64, String)>(
             "SELECT attribute_index, amount, source_card_id FROM owned_card_training \
-             WHERE owned_card_id = ? ORDER BY attribute_index",
+             WHERE owned_card_id = ?",
         )
         .bind(id)
         .fetch_all(pool)
@@ -421,20 +448,20 @@ mod tests {
                 "granted": 10,
                 "before": 0,
                 "after": 10,
+                "replaced": null,
             })
         );
         assert_eq!(
             training_of(&pool, "fresh").await,
-            vec![(4, 10, "def-card-1".to_string())]
+            vec![(Some(4), 10, "def-card-1".to_string())]
         );
         assert!(!source_exists(&pool, "card-1").await);
     }
 
-    /// Replacement-vs-stacking is UNKNOWN, so a second card on the SAME slot is
-    /// refused — and, critically, the refusal rolls back the whole transaction,
-    /// so the player keeps the card rather than paying for nothing.
+    /// A second card on the SAME slot REPLACES the first and does not
+    /// accumulate: 10 then 15 leaves 15, never 25.
     #[tokio::test]
-    async fn a_second_training_on_the_same_slot_is_refused_and_the_card_survives() {
+    async fn a_second_training_on_the_same_slot_replaces_rather_than_accumulating() {
         let (_dir, pool) = fixture().await;
         consume_item(
             &pool,
@@ -446,7 +473,7 @@ mod tests {
         .await
         .expect("first apply");
 
-        let err = consume_item(
+        let out = consume_item(
             &pool,
             "prof",
             "club",
@@ -454,22 +481,24 @@ mod tests {
             &train(4, 15),
         )
         .await
-        .expect_err("second apply on the same slot must be refused");
-        assert!(matches!(err, AppError::Conflict(_)), "got {err:?}");
+        .expect("second apply replaces");
 
-        // The original effect is untouched: not replaced, not stacked.
         assert_eq!(
             training_of(&pool, "fresh").await,
-            vec![(4, 10, "def-card-1".to_string())]
+            vec![(Some(4), 15, "def-card-2".to_string())],
+            "the newer effect must stand alone, not sum to 25"
         );
-        // And the second card was NOT spent.
-        assert!(source_exists(&pool, "card-2").await);
+        assert_eq!(out.effect["before"], json!(10));
+        assert_eq!(out.effect["after"], json!(15));
+        assert_eq!(out.effect["replaced"]["amount"], json!(10));
+        // Both cards were legitimately spent.
+        assert!(!source_exists(&pool, "card-2").await);
     }
 
-    /// Distinct slots are independent: each training card names exactly one
-    /// attribute, and nothing in the shipped data couples them.
+    /// A card on a DIFFERENT slot also replaces: FIFA 17 permits "one attribute
+    /// or all six", never a mixture, so two slots must never be boosted at once.
     #[tokio::test]
-    async fn distinct_slots_coexist_on_one_instance() {
+    async fn a_training_on_a_different_slot_still_replaces_the_previous_one() {
         let (_dir, pool) = fixture().await;
         for (identity, source, slot, amount) in
             [("act-1", "card-1", 4, 10), ("act-2", "card-2", 1, 5)]
@@ -486,13 +515,66 @@ mod tests {
         }
         assert_eq!(
             training_of(&pool, "fresh").await,
-            vec![
-                (1, 5, "def-card-2".to_string()),
-                (4, 10, "def-card-1".to_string())
-            ]
+            vec![(Some(1), 5, "def-card-2".to_string())],
+            "only the newest effect may remain"
         );
     }
 
+    /// The rare card boosts every slot, and replaces a single-attribute effect
+    /// exactly like any other new card.
+    #[tokio::test]
+    async fn the_all_six_card_stores_no_slot_and_replaces_a_single_attribute_effect() {
+        let (_dir, pool) = fixture().await;
+        consume_item(
+            &pool,
+            "prof",
+            "club",
+            &apply_request("act-1", "card-1", "fresh"),
+            &train(4, 15),
+        )
+        .await
+        .expect("single-attribute apply");
+
+        consume_item(
+            &pool,
+            "prof",
+            "club",
+            &apply_request("act-2", "card-2", "fresh"),
+            &train_all(10),
+        )
+        .await
+        .expect("all-six apply");
+
+        assert_eq!(
+            training_of(&pool, "fresh").await,
+            vec![(None, 10, "def-card-2".to_string())],
+            "the all-six effect stores a NULL slot and stands alone"
+        );
+    }
+
+    /// The all-six card authors at most +10; a caller declaring that ceiling
+    /// cannot then push +15 through it.
+    #[tokio::test]
+    async fn the_all_six_ceiling_is_enforced_independently() {
+        let (_dir, pool) = fixture().await;
+        let err = consume_item(
+            &pool,
+            "prof",
+            "club",
+            &apply_request("act-1", "card-1", "fresh"),
+            &InstanceEffect::ApplyTraining {
+                attribute_index: None,
+                amount: 15,
+                max_amount: 10,
+            },
+        )
+        .await
+        .expect_err("an over-ceiling all-six boost must be refused");
+        assert!(matches!(err, AppError::BadRequest(_)), "got {err:?}");
+        assert!(training_of(&pool, "fresh").await.is_empty());
+        assert!(source_exists(&pool, "card-1").await);
+    }
+
     /// The caller declares its own family's ceiling and is held to it. This is
     /// what stops a host describing a boost no card could grant.
     #[tokio::test]
@@ -516,7 +598,7 @@ mod tests {
     #[tokio::test]
     async fn a_slot_outside_the_card_model_is_refused() {
         let (_dir, pool) = fixture().await;
-        for slot in [-1, 6, 99] {
+        for slot in [-1i64, 6, 99] {
             let err = consume_item(
                 &pool,
                 "prof",
@@ -582,7 +664,7 @@ mod tests {
         assert_eq!(replay.effect, first.effect);
         assert_eq!(
             training_of(&pool, "fresh").await,
-            vec![(2, 15, "def-card-1".to_string())]
+            vec![(Some(2), 15, "def-card-1".to_string())]
         );
     }