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
|
// -------------------------------------------------------------- definitions
|
||||||
|
|
||||||
/// Definition-level (per-card) fields. Owned copies of one `resourceId` MUST
|
/// Definition-level fields carried per owned copy. The DEFINITION IDENTITY
|
||||||
/// agree on these; instance fields (wire id, contract, fitness, …) are excluded.
|
/// 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)]
|
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||||
pub struct DefFields {
|
pub struct DefFields {
|
||||||
pub asset_id: Option<i64>,
|
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
|
/// Build the definition proposal from player cards, applying the
|
||||||
/// resourceId-group consistency gate, entity resolution, and honest
|
/// resourceId-group consistency gate, entity resolution, and honest
|
||||||
/// buildability. `approved_conflicts` are the ONLY conflicts allowed to defer.
|
/// buildability. `approved_conflicts` are the ONLY conflicts allowed to defer.
|
||||||
@@ -347,12 +379,20 @@ pub fn plan_definitions(
|
|||||||
for (resource_id, items) in groups {
|
for (resource_id, items) in groups {
|
||||||
let wire_ids: Vec<i64> = items.iter().map(|i| i.id).collect();
|
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 first = def_fields(items[0]);
|
||||||
let mut distinct = vec![first.clone()];
|
let mut distinct = vec![first.clone()];
|
||||||
for it in &items[1..] {
|
for it in &items[1..] {
|
||||||
let f = def_fields(it);
|
let f = def_fields(it);
|
||||||
if !distinct.contains(&f) {
|
if !distinct
|
||||||
|
.iter()
|
||||||
|
.any(|d| DefIdentity::of(d) == DefIdentity::of(&f))
|
||||||
|
{
|
||||||
distinct.push(f);
|
distinct.push(f);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -319,6 +319,51 @@ fn emit_refuses_when_blocked() {
|
|||||||
assert!(emit_content(&rep, dir.path(), "x").is_err());
|
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
|
// ------------------------------------------------------------------ apply
|
||||||
|
|
||||||
use crate::apply::{
|
use crate::apply::{
|
||||||
|
|||||||
Reference in New Issue
Block a user