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.

migrations/0030_owned_card_training_single.sql

@@ -0,0 +1,66 @@
+-- Reshape attribute training to AT MOST ONE effect per instance, replaceable.
+--
+-- WHY THIS SUPERSEDES 0029'S SHAPE. 0029 keyed on (owned_card_id,
+-- attribute_index) and recorded that same-slot behaviour was UNKNOWN, enforcing
+-- the unknown as a refusal. That was the honest shape while the semantics were
+-- unrecovered. They are now recovered, and BOTH halves of 0029's shape are
+-- wrong:
+--
+--   * "You can only boost one attribute or all six. You can not do it with 2, 3,
+--     4 or 5 attributes." -- so two effects must never coexist on one instance,
+--     which the old composite key permitted (and which staging demonstrated by
+--     holding a slot-4 and a slot-1 effect at once).
+--   * "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 apply REPLACES, it does not refuse.
+--
+-- Both quotes are from the contemporaneous FIFA 17-specific training guide
+-- (fifauteam, published 2016-09-08), corroborated by the shipped table: each
+-- family has exactly 21 rows = 7 card types x 3 levels, and the 7th type in each
+-- family (subtypes 57 and 67) is the only one flagged `weightrare = 2` with
+-- amounts 3/6/10, matching the documented RARE "ALL" card at +3/+6/+10.
+-- DOCUMENTED, corroborated TABLE_PROVEN. It is NOT LIVE_PROVEN against EA.
+--
+-- 0029 is left intact rather than rewritten: it is already applied to the
+-- supervised staging environment, so migration history matters there.
+--
+-- NEW SHAPE. One row per instance, so "one attribute or all six" is a
+-- representable invariant instead of a convention:
+--   attribute_index INTEGER NULL -- a slot in Core's six-attribute model, or
+--                                   NULL meaning ALL SIX slots (the rare card).
+-- The PRIMARY KEY on owned_card_id alone is what makes a second application a
+-- REPLACE (delete-then-insert inside the one apply transaction) rather than an
+-- accumulation.
+--
+-- The 1..=15 amount bound is NOT tightened here: 15 is the single-attribute
+-- ceiling while the all-six card authors at most 10, and which ceiling applies
+-- depends on the card family -- a per-game rule that belongs at apply time where
+-- the game's table is in scope, not in the schema.
+--
+-- DATA CARRIED FORWARD: where an instance somehow holds several effects (only
+-- reachable on staging under 0029's shape), the MOST RECENT survives, which is
+-- exactly the "replaces" rule applied retroactively.
+
+CREATE TABLE owned_card_training_new (
+    owned_card_id   TEXT    NOT NULL PRIMARY KEY REFERENCES owned_cards(id) ON DELETE CASCADE,
+    attribute_index INTEGER          CHECK (attribute_index IS NULL OR attribute_index BETWEEN 0 AND 5),
+    amount          INTEGER NOT NULL CHECK (amount >= 1 AND amount <= 99),
+    source_card_id  TEXT    NOT NULL,
+    applied_at      TEXT    NOT NULL
+);
+
+INSERT INTO owned_card_training_new
+    (owned_card_id, attribute_index, amount, source_card_id, applied_at)
+SELECT t.owned_card_id, t.attribute_index, t.amount, t.source_card_id, t.applied_at
+  FROM owned_card_training t
+  JOIN (
+        SELECT owned_card_id, MAX(applied_at) AS newest
+          FROM owned_card_training
+         GROUP BY owned_card_id
+       ) pick
+    ON pick.owned_card_id = t.owned_card_id
+   AND pick.newest        = t.applied_at
+ GROUP BY t.owned_card_id;
+
+DROP TABLE owned_card_training;
+ALTER TABLE owned_card_training_new RENAME TO owned_card_training;

src/routes/cards.rs

@@ -147,15 +147,12 @@ pub async fn get_collection(
         let effective_overall = def.overall as i64 + o.training_bonus;
         let effective_position = o.position_override.as_deref().unwrap_or(&def.position);
         // Attribute training is per-instance state, so the finished attributes
-        // belong in the envelope beside the finished rating. The raw effects go
+        // belong in the envelope beside the finished rating. The raw effect goes
         // out too: a caller that needs to show WHICH attribute was trained
         // cannot recover that by differencing against a definition it may not
-        // have.
-        const NO_TRAINING: &[training_svc::TrainingEffect] = &[];
-        let effects = training
-            .get(&o.id)
-            .map(Vec::as_slice)
-            .unwrap_or(NO_TRAINING);
+        // have. At most ONE effect per instance -- FIFA 17 replaces rather than
+        // accumulates, so this is an Option, not a list.
+        let effect = training.get(&o.id);
         let body = json!({
             "owned_card_id": o.id,
             "content_kind": o.content_kind,
@@ -172,8 +169,8 @@ pub async fn get_collection(
             "contract_matches": o.contract_matches,
             "effective_overall": effective_overall,
             "effective_position": effective_position,
-            "effective_attributes": training_svc::effective_attributes_json(def, effects),
-            "training": effects,
+            "effective_attributes": training_svc::effective_attributes_json(def, effect),
+            "training": effect,
             "card": def,
         });
         views.push(OwnedItemView {

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())]
         );
     }
 

src/services/training.rs

@@ -27,11 +27,15 @@ use crate::{db::Pool, error::AppResult, models::card::CardDefinition};
 /// that keeps the projected card inside the model it is drawn from.
 pub const ATTRIBUTE_MAX: i64 = 99;
 
-/// One training effect attached to one instance.
+/// The one training effect an instance may carry.
+///
+/// At most one per instance: FIFA 17 allows "one attribute or all six" and a new
+/// card replaces the old, so a second concurrent effect is not representable.
 #[derive(Debug, Clone, Serialize, FromRow)]
 pub struct TrainingEffect {
-    /// Slot in Core's six-attribute model, `CardDefinition` declaration order.
-    pub attribute_index: i64,
+    /// Slot in Core's six-attribute model, `CardDefinition` declaration order,
+    /// or `None` for an effect that boosts ALL SIX slots.
+    pub attribute_index: Option<i64>,
     pub amount: i64,
     pub source_card_id: String,
 }
@@ -44,30 +48,30 @@ pub struct TrainingEffect {
 pub async fn load_for_club(
     pool: &Pool,
     club_id: &str,
-) -> AppResult<HashMap<String, Vec<TrainingEffect>>> {
-    let rows = sqlx::query_as::<_, (String, i64, i64, String)>(
+) -> AppResult<HashMap<String, TrainingEffect>> {
+    let rows = sqlx::query_as::<_, (String, Option<i64>, i64, String)>(
         "SELECT t.owned_card_id, t.attribute_index, t.amount, t.source_card_id \
            FROM owned_card_training t \
            JOIN owned_cards o ON o.id = t.owned_card_id \
-          WHERE o.club_id = ? \
-          ORDER BY t.owned_card_id, t.attribute_index",
+          WHERE o.club_id = ?",
     )
     .bind(club_id)
     .fetch_all(pool)
     .await?;
 
-    let mut by_instance: HashMap<String, Vec<TrainingEffect>> = HashMap::new();
-    for (owned_card_id, attribute_index, amount, source_card_id) in rows {
-        by_instance
-            .entry(owned_card_id)
-            .or_default()
-            .push(TrainingEffect {
-                attribute_index,
-                amount,
-                source_card_id,
-            });
-    }
-    Ok(by_instance)
+    Ok(rows
+        .into_iter()
+        .map(|(owned_card_id, attribute_index, amount, source_card_id)| {
+            (
+                owned_card_id,
+                TrainingEffect {
+                    attribute_index,
+                    amount,
+                    source_card_id,
+                },
+            )
+        })
+        .collect())
 }
 
 /// The definition's six attributes in canonical slot order.
@@ -88,14 +92,23 @@ pub fn base_attributes(def: &CardDefinition) -> [i64; 6] {
 
 /// Base attributes with any training folded in, clamped to the model's domain.
 ///
+/// A `None` slot boosts ALL SIX attributes — FIFA 17's rare "all" training card.
 /// An out-of-range slot is ignored rather than panicking: the schema already
 /// refuses one, so reaching this would mean the row was written around Core, and
 /// dropping it degrades one attribute instead of failing every projection.
-pub fn effective_attributes(def: &CardDefinition, effects: &[TrainingEffect]) -> [i64; 6] {
+pub fn effective_attributes(def: &CardDefinition, effect: Option<&TrainingEffect>) -> [i64; 6] {
     let mut out = base_attributes(def);
-    for e in effects {
-        if let Some(slot) = out.get_mut(e.attribute_index as usize) {
-            *slot = (*slot + e.amount).clamp(0, ATTRIBUTE_MAX);
+    let Some(e) = effect else { return out };
+    match e.attribute_index {
+        Some(slot) => {
+            if let Some(v) = out.get_mut(slot as usize) {
+                *v = (*v + e.amount).clamp(0, ATTRIBUTE_MAX);
+            }
+        }
+        None => {
+            for v in out.iter_mut() {
+                *v = (*v + e.amount).clamp(0, ATTRIBUTE_MAX);
+            }
         }
     }
     out
@@ -104,9 +117,9 @@ pub fn effective_attributes(def: &CardDefinition, effects: &[TrainingEffect]) ->
 /// The same six values as a named object, for the projection envelope.
 pub fn effective_attributes_json(
     def: &CardDefinition,
-    effects: &[TrainingEffect],
+    effect: Option<&TrainingEffect>,
 ) -> serde_json::Value {
-    let a = effective_attributes(def, effects);
+    let a = effective_attributes(def, effect);
     serde_json::json!({
         "pace": a[0],
         "shooting": a[1],