fix(import-fifa17): nation/league/team are instance metadata, not definition identity
Evidence (resourceId 169193): its 4 owned copies are IDENTICAL in asset/rating/ position/all attributes and differ ONLY in nation/team/league (and those resolve inconsistently, e.g. team 240 'Atletico Madrid' under league 16 'Ligue 1'). A player's club affiliation is an instance-time snapshot, not part of the card DEFINITION identity. Correct the model (not a special-case): the definition-consistency gate now compares a DefIdentity projection (asset_id/version/rating/position/attrs/ rareflag) and EXCLUDES nation/league/team. A club-only difference between copies of one resourceId is no longer a conflict; a real identity disagreement (rating/position/attrs/asset) still trips it. The definition's display nation/league/club use the first-observed copy (deterministic; display-only, never identity). No --defer-conflict allowlist entry is needed for 169193 now. On the real profile: conflicts 1->0, 169193 reclassified conflict->NoName (still deferred, unnameable), supported still 1681, deferred instances still 13, BLOCKERS none without any --defer-conflict flag. 2 new tests (club-only diff is not a conflict; rating diff still is). crate suite 25 green; clippy -D clean.
This commit is contained in:
@@ -215,8 +215,13 @@ pub fn count_items(profile: &Profile) -> ItemCounts {
|
||||
|
||||
// -------------------------------------------------------------- definitions
|
||||
|
||||
/// Definition-level (per-card) fields. Owned copies of one `resourceId` MUST
|
||||
/// agree on these; instance fields (wire id, contract, fitness, …) are excluded.
|
||||
/// Definition-level fields carried per owned copy. The DEFINITION IDENTITY
|
||||
/// subset (`asset_id`/`version`/`rating`/`position`/`attrs`/`rareflag`) MUST
|
||||
/// agree across copies of one `resourceId`; `nation`/`league`/`team` are
|
||||
/// observed INSTANCE metadata that MAY legitimately vary per owned copy (a
|
||||
/// club-affiliation snapshot — evidence: `resourceId` 169193) and are EXCLUDED
|
||||
/// from the identity gate. Instance fields (wire id, contract, fitness, …) are
|
||||
/// not modelled here at all.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct DefFields {
|
||||
pub asset_id: Option<i64>,
|
||||
@@ -327,6 +332,33 @@ fn def_fields(item: &Item) -> DefFields {
|
||||
}
|
||||
}
|
||||
|
||||
/// The definition-IDENTITY projection of [`DefFields`]: the subset that MUST be
|
||||
/// identical across every owned copy of one `resourceId`. Deliberately omits
|
||||
/// `nation`/`league`/`team`, which are observed instance metadata (see
|
||||
/// [`DefFields`]) — a club-only difference between two copies is NOT a conflict.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
struct DefIdentity {
|
||||
asset_id: Option<i64>,
|
||||
version: i64,
|
||||
rating: Option<i64>,
|
||||
position: Option<String>,
|
||||
attrs: Option<Vec<i64>>,
|
||||
rareflag: Option<i64>,
|
||||
}
|
||||
|
||||
impl DefIdentity {
|
||||
fn of(f: &DefFields) -> Self {
|
||||
DefIdentity {
|
||||
asset_id: f.asset_id,
|
||||
version: f.version,
|
||||
rating: f.rating,
|
||||
position: f.position.clone(),
|
||||
attrs: f.attrs.clone(),
|
||||
rareflag: f.rareflag,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Build the definition proposal from player cards, applying the
|
||||
/// resourceId-group consistency gate, entity resolution, and honest
|
||||
/// buildability. `approved_conflicts` are the ONLY conflicts allowed to defer.
|
||||
@@ -347,12 +379,20 @@ pub fn plan_definitions(
|
||||
for (resource_id, items) in groups {
|
||||
let wire_ids: Vec<i64> = items.iter().map(|i| i.id).collect();
|
||||
|
||||
// 1) consistency gate.
|
||||
// 1) consistency gate — compare DEFINITION IDENTITY only. A club-only
|
||||
// difference (nation/league/team) between copies is observed instance
|
||||
// metadata, never a conflict (evidence: resourceId 169193's 4 copies are
|
||||
// identical but for club). `distinct` keeps only identity-distinct
|
||||
// variants so a real conflict (differing rating/position/attrs/asset)
|
||||
// still trips the gate.
|
||||
let first = def_fields(items[0]);
|
||||
let mut distinct = vec![first.clone()];
|
||||
for it in &items[1..] {
|
||||
let f = def_fields(it);
|
||||
if !distinct.contains(&f) {
|
||||
if !distinct
|
||||
.iter()
|
||||
.any(|d| DefIdentity::of(d) == DefIdentity::of(&f))
|
||||
{
|
||||
distinct.push(f);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -319,6 +319,51 @@ fn emit_refuses_when_blocked() {
|
||||
assert!(emit_content(&rep, dir.path(), "x").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn club_only_difference_across_copies_is_not_a_conflict() {
|
||||
// Two owned copies of one resourceId, identical in asset/rating/position/
|
||||
// attributes but DIFFERENT nation/team/league (a club-affiliation snapshot).
|
||||
// Evidence (resourceId 169193) says this is NOT a conflict: one definition,
|
||||
// both preserved wire ids, display club from the first-observed copy.
|
||||
let a = format!(
|
||||
r#"{{"id":100000001,"resourceId":20801,"assetId":20801,"itemType":"player","rareflag":1,
|
||||
"rating":94,"preferredPosition":"ST","nation":38,"teamid":243,"leagueId":53,"attributeList":{}}}"#,
|
||||
attrs()
|
||||
);
|
||||
let b = format!(
|
||||
r#"{{"id":100000002,"resourceId":20801,"assetId":20801,"itemType":"player","rareflag":1,
|
||||
"rating":94,"preferredPosition":"ST","nation":45,"teamid":21,"leagueId":16,"attributeList":{}}}"#,
|
||||
attrs()
|
||||
);
|
||||
let plan = defs(&[a, b]);
|
||||
assert!(
|
||||
plan.conflicts.is_empty(),
|
||||
"a club-only difference must NOT be a conflict"
|
||||
);
|
||||
assert_eq!(plan.supported.len(), 1, "one definition");
|
||||
let d = &plan.supported[0];
|
||||
assert_eq!(
|
||||
d.wire_ids,
|
||||
vec![100000001, 100000002],
|
||||
"both copies preserved"
|
||||
);
|
||||
assert_eq!(d.nation, "Portugal", "display club = first observed copy");
|
||||
assert_eq!(d.club, "Real Madrid");
|
||||
assert_eq!(d.league, "LaLiga");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn identity_difference_across_copies_is_still_a_conflict() {
|
||||
// A differing rating is a real definition-identity disagreement -> conflict,
|
||||
// never silently merged.
|
||||
let plan = defs(&[
|
||||
player(100000001, 20801, 20801, 94),
|
||||
player(100000002, 20801, 20801, 90),
|
||||
]);
|
||||
assert!(plan.supported.is_empty());
|
||||
assert_eq!(plan.conflicts.len(), 1);
|
||||
}
|
||||
|
||||
// ------------------------------------------------------------------ apply
|
||||
|
||||
use crate::apply::{
|
||||
|
||||
Reference in New Issue
Block a user