diff --git a/.gitignore b/.gitignore index aeffcb3..15ef6c4 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,9 @@ build/ venv/ env/ +# uv resolver lockfile: local environment setup only, not a project workflow +uv.lock + # IDE .vscode/ .idea/ diff --git a/src/gpcr_tools/aggregator/runner.py b/src/gpcr_tools/aggregator/runner.py index 2787498..f1ab592 100644 --- a/src/gpcr_tools/aggregator/runner.py +++ b/src/gpcr_tools/aggregator/runner.py @@ -49,6 +49,8 @@ ALERT_PREFIX_ALPHA5_GRAFT, ALERT_PREFIX_API_UNAVAILABLE, ALERT_PREFIX_CHIMERIC_REVIEW, + ALERT_PREFIX_GALPHA_SPECIES_UNVERIFIED, + ALERT_PREFIX_GALPHA_SUBTYPE_UNRESOLVED, ALERT_PREFIX_HALLUCINATION, ALERT_PREFIX_TIE_BREAKER_ALIGNED, ALERT_PREFIX_TIE_BREAKER_OVERRIDE, @@ -60,6 +62,7 @@ CHIMERA_SUBTYPE_LOW_CONFIDENCE, EMPTY_VALUES, FULL_G_ALPHA_CANDIDATES, + LIST_ITEM_KEY_FIELDS, LOW_CONFIDENCE_LEVELS, POLYMER_FEATURES_CACHE_NAME, SITE_REF_UNKNOWN, @@ -611,6 +614,263 @@ def _chains(copies: list[dict[str, Any]]) -> str: best_run_data["ligand_copies"] = copy.deepcopy(voted_copies) +def _multi_copy_alert_component(alert: dict[str, Any]) -> str | None: + """Component id a ``MULTI_COPY_LIGAND`` alert refers to, from its message path. + + The alert message carries the component in a ``ligands[]`` path (the + same anchor the excluded-buffer prune keys on). Returns the component id, or + ``None`` when no such path is present. + """ + message = str(alert.get("message", "")) + marker = "ligands[" + start = message.find(marker) + if start == -1: + return None + start += len(marker) + end = message.find("]", start) + if end == -1: + return None + return message[start:end] + + +def _multi_copy_site_divergence(sites: list[Any] | None) -> bool: + """Whether a component's per-copy binding sites diverge enough to gate review. + + Gates (returns ``True``) iff the joined copies place the component at more than + one distinct binding site. Fail-closed: an absent / unattributable site is + treated as its own distinct value (so a known-vs-blank split gates), and fewer + than two joinable copies also gates -- a copy count the alert saw but the + per-copy attribution cannot corroborate is left for a curator, not waved + through. Same site across every copy is advisory (returns ``False``). + """ + if not sites or len(sites) < 2: + return True + return len({_copy_token(s) for s in sites}) > 1 + + +def _mark_multi_copy_ligand_gating(best_run_data: dict[str, Any]) -> None: + """Stamp each ``MULTI_COPY_LIGAND`` oligomer alert with a ``gating`` flag. + + A component modelled in several copies only warrants a curator's stop when those + copies sit at DISTINCT binding sites; copies that all share one site are an + advisory the curator still sees but that does not gate acceptance. The per-copy + binding sites come from the aggregated ``ligand_copies`` list, joined to each + component through the oligomer roster's ``nonpolymer_instance_index``. The flag + the read-time gate reads (:func:`gpcr_tools.validator.gating.oligomer_gating_warnings`) + is written here, in place, per :func:`_multi_copy_site_divergence`. + + Must run AFTER :func:`_rebuild_small_molecule_rows_from_per_copy`, which sets the + final aggregated ``ligand_copies`` this join reads. Alerts of any other type are + untouched; a record with no oligomer analysis or no such alerts is a no-op. + """ + oligomer = best_run_data.get("oligomer_analysis") + if not isinstance(oligomer, dict): + return + alerts = oligomer.get("alerts") + if not isinstance(alerts, list): + return + + # Reverse index: per-copy identifier (":") -> component + # id, from the oligomer roster -- the same token the aggregated ``ligand_copies`` + # rows carry in ``copy_id`` (see _rebuild_small_molecule_rows_from_per_copy). + instance_index = oligomer.get("nonpolymer_instance_index") + token_to_comp: dict[str, str] = {} + if isinstance(instance_index, dict): + for comp_id, instances in instance_index.items(): + if not isinstance(comp_id, str) or not isinstance(instances, list): + continue + for inst in instances: + if not isinstance(inst, dict): + continue + token = f"{_copy_token(inst.get('auth_asym_id'))}:{_copy_token(inst.get('auth_seq_id'))}" + token_to_comp[token] = comp_id + + # Per-component list of the binding sites its joined copies were attributed to. + sites_by_comp: dict[str, list[Any]] = {} + for copy_row in best_run_data.get("ligand_copies") or []: + if not isinstance(copy_row, dict): + continue + comp_id = token_to_comp.get(_copy_token(copy_row.get("copy_id"))) + if comp_id is None: + continue + sites_by_comp.setdefault(comp_id, []).append(copy_row.get("site_ref")) + + for alert in alerts: + if not isinstance(alert, dict) or alert.get("type") != ALERT_MULTI_COPY_LIGAND: + continue + comp_id = _multi_copy_alert_component(alert) + sites = sites_by_comp.get(comp_id) if comp_id is not None else None + alert["gating"] = _multi_copy_site_divergence(sites) + + +def _shipped_base_prefixes(best_run_data: dict[str, Any]) -> set[str]: + """Open (unclosed-bracket) path prefixes of every shipped entity's BASE identity. + + Each prefix is ``field[`` -- deliberately WITHOUT the closing + ``]`` -- so a boundary test can tell where the identity ends. The base identity + is built through the shared config helper (:func:`list_item_identity`) so it is + byte-identical to the one a discrepancy path is built from; for the ligands + list any ``site_ref`` is removed from the item BEFORE the helper runs, because + the ligand identity embeds ``site_ref`` (``comp:site``) and we must reconcile + at the base-compound level: a compound that still ships at a NEW site is not a + dropped entity. ``site_ref`` is dropped via a filtered copy through the shared + helper -- never a regex strip -- so a bracketed peptide name is left intact. + Auxiliary-protein and per-copy identities carry no ``site_ref`` suffix and pass + through unchanged. + """ + prefixes: set[str] = set() + for list_field, key_field in LIST_ITEM_KEY_FIELDS.items(): + items = best_run_data.get(list_field) + if not isinstance(items, list): + continue + for idx, item in enumerate(items): + if not isinstance(item, dict): + continue + base_item = item + if key_field == "chem_comp_id" and "site_ref" in item: + # Base-compound identity: strip site_ref so the same compound at a + # different site is not treated as a different (dropped) entity. + base_item = {k: v for k, v in item.items() if k != "site_ref"} + base = list_item_identity(base_item, key_field, idx) + prefixes.add(f"{list_field}[{base}") + return prefixes + + +def _path_covered(path: str, open_prefixes: set[str]) -> bool: + """Whether some shipped BASE prefix covers *path* at a structural boundary. + + *open_prefixes* are ``field[`` strings with no closing bracket + (see :func:`_shipped_base_prefixes`). A prefix covers *path* only when *path* + continues with a boundary character immediately after the base identity: + + * ``:`` -- a ``site_ref`` suffix follows in the ligand identity + (``ligands[HEM:allosteric]...``); or + * ``]`` -- the identity closes (no site, or an aux / per-copy identity). + + BRACKET-SAFE: the prefix is built from shipped data and the path is matched + against it -- the path is NEVER split on ``[`` / ``]``. So a compound id that + is a strict substring of a longer id (``HEM`` vs ``HEME``), and a peptide name + containing brackets whose lookalike sibling differs only in a trailing token + (``[Sar1,Ile8]-Angiotensin II`` vs ``... III``), both FAIL the boundary test + instead of matching. Reconciling at the base-compound level means a controversy + on a compound that still ships at ANY site stays covered (keeps gating); only a + compound dropped from the record entirely goes uncovered. + """ + for prefix in open_prefixes: + if path.startswith(prefix) and path[len(prefix) : len(prefix) + 1] in (":", "]"): + return True + return False + + +def _assert_boundary_matcher_sound(open_prefixes: set[str]) -> None: + """Fail-closed self-check that the bracket-safe boundary matcher actually works. + + A vacuous guard proves nothing. Two ways this check could have been vacuous, + both closed here: + + 1. A guard that only runs its discriminating probes inside a + ``for prefix in open_prefixes`` loop proves NOTHING when nothing shipped + (empty set) -- yet that is exactly the case where every list-path + controversy gets downgraded, so the matcher must be sound there too. So the + discriminating probes below are FIXED and run UNCONDITIONALLY, independent of + what shipped. + 2. The one boundary that actually protects a real gate is ``:`` -- the + pre-vs-post-rebuild path mismatch. A step-10c rebuild can move a still- + shipping compound to a new site, so a controversy recorded PRE-rebuild reads + ``ligands[HEM:orthosteric].site_ref`` while the shipped base prefix is + ``ligands[HEM`` (site stripped). Reconcile must KEEP gating that genuine + site conflict, which requires the matcher to cover the ``:`` continuation. + A matcher that handled only ``]`` (identity closes immediately) would pass a + self-check that never probes ``:`` yet silently clear that real gate. So we + assert BOTH boundaries positively. + + A regression in any direction (bare ``startswith`` that ignores the boundary; a + matcher that accepts only ``]`` and drops ``:``; or one that accepts only ``:`` + and drops ``]``) trips an assertion here and routes the PDB to human review + rather than clearing a gate. + """ + # Fixed probes -- ALWAYS run, so the guard is non-vacuous even when nothing + # shipped. ``base`` stands in for any shipped base identity ``field[``. + base = "ligands[HEM" + # Positive, ``]`` boundary: the base identity closes immediately (no site). + assert _path_covered(f"{base}].pubchem_id", {base}), ( + "boundary matcher failed to cover a base identity at its closing bracket" + ) + # Positive, ``:`` boundary: the site-qualified descendant -- the exact + # pre-vs-post-rebuild path (HEM:orthosteric under a shipped HEM) whose genuine + # site conflict reconcile must keep gating. A matcher blind to ``:`` fails HERE. + assert _path_covered(f"{base}:orthosteric].site_ref", {base}), ( + "boundary matcher failed to cover a site-qualified descendant (':' boundary)" + ) + # Negative, name-char continuation: a longer id sharing the base as a strict + # prefix (HEME under HEM) MUST NOT match. A bare-startswith matcher fails HERE. + assert not _path_covered(f"{base}E:orthosteric].site_ref", {base}), ( + "boundary matcher matched a longer id (HEME) under a shorter one (HEM)" + ) + # Bracketed peptide identity: the path is never naively split on ``[`` / ``]``. + canary = "ligands[__keyless__:[sar1,ile8] angiotensin ii" + sibling = "ligands[__keyless__:[sar1,ile8] angiotensin iii" + assert _path_covered(f"{canary}].site_ref", {canary}), ( + "boundary matcher failed on a bracketed peptide identity" + ) + # The lookalike sibling (III) shares the shipped II identity as a strict prefix + # followed by a name char; a bare-startswith matcher would match it HERE. + assert not _path_covered(f"{sibling}].site_ref", {canary}), ( + "boundary matcher matched a lookalike bracketed sibling (III under II)" + ) + + # Every real shipped prefix must behave the same as the fixed probes. + for prefix in open_prefixes: + assert _path_covered(f"{prefix}].site_ref", open_prefixes), ( + f"boundary matcher failed to cover shipped prefix {prefix!r}" + ) + assert _path_covered(f"{prefix}:orthosteric].site_ref", open_prefixes), ( + f"boundary matcher failed to cover site-qualified descendant of {prefix!r}" + ) + assert not _path_covered(f"{prefix}X].site_ref", {prefix}), ( + f"boundary matcher matched a longer id for prefix {prefix!r}" + ) + + +def _reconcile_source_discrepancies( + best_run_data: dict[str, Any], + discrepancies: list[dict[str, Any]], +) -> None: + """Downgrade (in place) gating controversies no shipped entity can own. + + A vote controversy on a ``ligands[...]`` / ``auxiliary_proteins[...]`` / + ``ligand_copies[...]`` path can only encode a shipped error if some shipped + entity actually lives under that path. When aggregation rebuilds or drops the + list, a controversy can be left pointing at an entity the final record no + longer carries; that controversy has nothing to gate, so it becomes advisory. + + Reconciliation is at the BASE-COMPOUND level (see :func:`_shipped_base_prefixes`): + a ligand identity embeds ``site_ref``, and a step-10c rebuild can rewrite a + still-shipping compound's site. Matching on the full site-qualified identity + would then read that compound as "dropped" and clear a GENUINE site conflict on + a compound that still ships -- exactly the confidently-wrong-released class the + gate exists to catch. So a controversy stays gating whenever its base compound + still ships at ANY site; only an entity dropped from the record entirely is + downgraded. + + Fail-closed: the bracket-safe boundary matcher is validated first + (:func:`_assert_boundary_matcher_sound`); if it is ever untrustworthy we assert + rather than risk clearing a real gate -- the raised error routes the PDB to + human review. + """ + prefixes = _shipped_base_prefixes(best_run_data) + _assert_boundary_matcher_sound(prefixes) + + for record in discrepancies: + path = record.get("path") + if not isinstance(path, str) or not record.get("gating", True): + continue + if not any(path.startswith(f"{list_field}[") for list_field in LIST_ITEM_KEY_FIELDS): + continue + if not _path_covered(path, prefixes): + record["gating"] = False + + def _build_validation_report( pdb_id: str, best_run_data: dict[str, Any], @@ -618,6 +878,7 @@ def _build_validation_report( all_warnings: list[str], chimera_result: dict[str, Any], validation_cache: ValidationCache | None, + ligand_advisories: list[str] | None = None, ) -> dict[str, Any]: """Assemble the validation report from all warning sources. @@ -625,11 +886,15 @@ def _build_validation_report( status comparisons go through the shared constants. The G-alpha sequence finding is classified against the model's claim: family agreement, subtype resolution, and routing of an indistinguishable subtype to human review. + + ``ligand_advisories`` are non-gating ligand findings (see + :func:`validate_and_enrich_ligands`); they are recorded as detector notes so + the curator sees them without the PDB being held for review. """ report: dict[str, Any] = { "critical_warnings": list(all_warnings), "algo_conflicts": [], - "detector_notes": [], + "detector_notes": list(ligand_advisories or []), "chimera_score": chimera_result.get("score") or 0, "chimera_status": chimera_result.get("status") or CHIMERA_STATUS_SKIPPED, "timestamp": datetime.now(tz=UTC).isoformat(), @@ -637,16 +902,21 @@ def _build_validation_report( # G protein subunit fragments the model misfiled under auxiliary_proteins / # ligands (a GaCT / alpha5 peptide, or a tag-named beta/gamma subunit). An - # unambiguous single-subunit fragment is MOVED into the G protein record (a - # gating warning routes the move to a curator); a no-slug or cross-role fragment - # is gated in place. This mutates the ligands / auxiliary_proteins lists, so it - # must run BEFORE the integrity check's positional list-index recursion + # unambiguous single-subunit fragment is MOVED into the G protein record; a + # no-slug or cross-role fragment is gated in place. A recovered ALPHA subunit + # (Galpha identity axis) and every MISFILED variant GATE for a curator; an + # authoritative beta / gamma recovery is an advisory detector note (matching + # crystallization fusions, binder renames, and ligand advisories, which all + # ship as detector_notes). This mutates the ligands / auxiliary_proteins lists, + # so it must run BEFORE the integrity check's positional list-index recursion # (validate_all emits 'ligands[N]' paths that curate parses into index cleanups), # exactly as the excluded-buffer prune (step 10b) precedes this report for the # same reason. - report["critical_warnings"].extend( - relocate_misfiled_g_protein_fragments(enriched_entry, best_run_data) + g_protein_gating, g_protein_advisory = relocate_misfiled_g_protein_fragments( + enriched_entry, best_run_data ) + report["critical_warnings"].extend(g_protein_gating) + report["detector_notes"].extend(g_protein_advisory) # Integrity checks (ghost chain, fake UniProt/PubChem, ghost ligand, method) integrity_warnings = validate_all(pdb_id, best_run_data, enriched_entry, cache=validation_cache) @@ -755,12 +1025,27 @@ def _build_validation_report( f"alpha5 '{a5_tail}' resolves G-alpha to '{subtype}'." ) elif resolution == CHIMERA_SUBTYPE_LOW_CONFIDENCE: + # The alpha5 window matched too weakly to say anything about the G-alpha. + # That is the one branch where the sequence check -- the only independent + # evidence about G-alpha identity -- returns nothing, so whatever the + # model asserted here ships unverified. Route it by what the model did: + # an asserted slug is an identity claim with no evidence behind it and + # gates; an honest abstention claims nothing, so it stays a note. subtype_basis = SUBTYPE_BASIS_CONSTRUCT_NAME - report["detector_notes"].append( - f"{ALERT_PREFIX_ALGO_WARNING} at 'chimera_analysis': " - f"alpha5 match is weak (best window score " - f"{chimera_result.get('score') or 0}); G-alpha identity unverified." - ) + weak_score = chimera_result.get("score") or 0 + if ai_uniprot: + report["algo_conflicts"].append( + f"{ALERT_PREFIX_ALGO_WARNING} at 'chimera_analysis': " + f"alpha5 match is weak (best window score {weak_score}), so the " + f"model's G-alpha '{ai_uniprot}' could not be verified against the " + f"sequence. Confirm the identity." + ) + else: + report["detector_notes"].append( + f"{ALERT_PREFIX_ALGO_WARNING} at 'chimera_analysis': " + f"alpha5 match is weak (best window score {weak_score}); no G-alpha " + f"identity was asserted, so there is nothing to verify." + ) elif ai_family and family and ai_family != family: # The model's family disagrees with the alpha5 coupling family. subtype_basis = SUBTYPE_BASIS_CONSTRUCT_NAME @@ -784,14 +1069,52 @@ def _build_validation_report( subtype_basis = SUBTYPE_BASIS_CONSTRUCT_NAME members = ", ".join(candidate_set) or "indistinguishable subtypes" off_roster = [s for s in candidate_set if s not in _RECOGNISED_G_ALPHA_SLUGS] + # Whether this is a native, family-consistent G protein whose only + # residual ambiguity is the structurally-inseparable subtype. The + # downgrade to an advisory note is keyed on DETERMINISTIC signals + # only -- never on the model's is_chimeric flag alone, which the + # model can silently omit. All four must hold: + # - the family is verified (model family == alpha5 family); + # - the model did not itself declare a chimera; + # - the deposited backbone is a single family-consistent slug + # (backbone family == alpha5 family), i.e. not a construct built + # on a foreign scaffold and not an entity with no attached slug; + # - there is no alpha5-graft signature (a grafted foreign alpha5). + # A native inseparable-subtype call (gnai1/gnai2, gnaq/gna11, the + # transducins) is then advisory: the specific member simply cannot be + # read from structure, so forcing a curator to pick one is noise. + backbone_family = chimera_result.get("backbone_family") + backbone_slug = chimera_result.get("backbone_slug") + native_family_consistent = ( + subtype_basis == SUBTYPE_BASIS_FAMILY_VERIFIED + and g_protein.get("is_chimeric") is not True + and backbone_slug is not None + and backbone_family == family + and not chimera_result.get("is_alpha5_graft") + ) if off_roster: + # Non-human ortholog of a verified family: a species / GPCRdb + # mapping question, still gating. report["critical_warnings"].append( - f"{ALERT_PREFIX_CHIMERIC_REVIEW} at " + f"{ALERT_PREFIX_GALPHA_SPECIES_UNVERIFIED} at " f"'signaling_partners.g_protein.alpha_subunit': alpha5 indicates a " f"non-human ortholog of the {family} family ({members}); confirm " f"the species / GPCRdb mapping." ) + elif native_family_consistent: + # Native, family-consistent G protein whose subtype is + # structurally inseparable: advisory note, does not gate. + report["detector_notes"].append( + f"{ALERT_PREFIX_GALPHA_SUBTYPE_UNRESOLVED} at " + f"'signaling_partners.g_protein.alpha_subunit': alpha5 confirms the " + f"{family} family; the specific subtype ({members}) has an identical " + f"alpha5 and cannot be resolved from structure. Native {family} G " + f"protein -- advisory." + ) else: + # Family verified only by construct name, or some other reason + # the family-consistent-native test did not hold: keep the + # gating chimera review so a curator confirms the subtype. report["critical_warnings"].append( f"{ALERT_PREFIX_CHIMERIC_REVIEW} at " f"'signaling_partners.g_protein.alpha_subunit': alpha5 confirms the " @@ -1099,11 +1422,17 @@ def aggregate_pdb( # 6. Ligand validation (mutates best_run_data, returns warnings) all_warnings: list[str] = [] + # Advisory ligand findings (an already-blanked PubChem CID, an apo + # placeholder whose only companions are structural cofactors/ions/lipids) + # are collected separately so the report records them as detector notes + # rather than gating warnings. + ligand_advisories: list[str] = [] ligand_warnings = validate_and_enrich_ligands( pdb_id, best_run_data, enriched, synonym_cache=synonym_cache if not skip_api_checks else None, + advisory_notes=ligand_advisories, ) all_warnings.extend(ligand_warnings) @@ -1173,6 +1502,22 @@ def aggregate_pdb( # before those paths are produced. _rebuild_small_molecule_rows_from_per_copy(best_run_data, majority_votes) + # 10d. Decide, per multi-copy-ligand alert, whether it gates. A component + # modelled in several copies only stops the curator when those copies sit at + # DISTINCT binding sites; copies that all share one site stay advisory. Runs + # AFTER the per-copy rebuild (step 10c) so it reads the final aggregated + # ``ligand_copies`` the join depends on, and BEFORE the validation report / + # write so the flag is persisted for the read-time gate. + _mark_multi_copy_ligand_gating(best_run_data) + + # 10e. Source-side reconcile: now that the ligand list is in its final + # shape, downgrade any gating controversy whose path no shipped entity + # covers (a rebuilt/dropped row can leave a controversy pointing at an + # identity the record no longer carries). Runs after the rebuild so + # reachability is checked against the shipped identities, and before the + # report so the gate reflects the reconciled controversies. + _reconcile_source_discrepancies(best_run_data, discrepancies) + # 11. Assemble validation report v_cache = validation_cache if not skip_api_checks else None report = _build_validation_report( @@ -1182,6 +1527,7 @@ def aggregate_pdb( all_warnings, chimera_result, v_cache, + ligand_advisories=ligand_advisories, ) # 12. Atomic write block diff --git a/src/gpcr_tools/aggregator/voting.py b/src/gpcr_tools/aggregator/voting.py index b7ca37d..837153b 100644 --- a/src/gpcr_tools/aggregator/voting.py +++ b/src/gpcr_tools/aggregator/voting.py @@ -11,9 +11,11 @@ from typing import Any from gpcr_tools.config import ( + APO_SENTINEL, CASE_FOLD_NAME_FIELDS, GROUND_TRUTH_PATHS, LIST_ITEM_KEY_FIELDS, + SITE_REF_UNKNOWN, SOFT_FIELD_KEYS, VOTE_NEAR_TIE_MARGIN, is_empty_key, @@ -22,6 +24,33 @@ list_item_identity as _list_item_identity, ) +# The keyless grouping identity of an apo (no-ligand) placeholder, as it appears +# inside a discrepancy path: ``ligands[__keyless__:apo].site_ref``. The closing +# bracket is part of the match so a real component whose normalized name merely +# starts with "apo" (e.g. an "apocynin" ligand) is never mistaken for it. +_APO_PLACEHOLDER_SEGMENT = f"[__keyless__:{APO_SENTINEL}]" + + +def _site_ref_controversy_is_non_gating(path: str, shipped_value: Any) -> bool: + """Whether a ``site_ref`` vote controversy should be advisory, not gating. + + A binding-site disagreement stops encoding a shipped error in exactly two + cases, and no others: + + * the controversy is on the apo (no-ligand) placeholder, whose ``site_ref`` + is meaningless and never reaches ligands.csv; or + * the shipped value is literally ``unknown`` -- the record committed to no + site, so there is no wrong site to review. + + A near-tie between two REAL sites (orthosteric vs allosteric), or the mere + presence of ``unknown`` among the votes while a real site was shipped, is a + genuine site conflict and keeps gating. + """ + if _APO_PLACEHOLDER_SEGMENT in path: + return True + return str(shipped_value).strip().lower() == SITE_REF_UNKNOWN + + # --------------------------------------------------------------------------- # Utility helpers # --------------------------------------------------------------------------- @@ -435,6 +464,14 @@ def find_discrepancies( # also keeps gating. if current_key == "role" and path.startswith("ligand_copies["): record["gating"] = False + # A site_ref controversy is advisory when it is on the apo placeholder + # or the shipped value is 'unknown' (no committed site to be wrong). + # Real site conflicts (a shipped orthosteric/allosteric value) keep + # gating. + if current_key == "site_ref" and _site_ref_controversy_is_non_gating( + path, best_run_data + ): + record["gating"] = False discrepancies.append(record) else: margin = _vote_margin(all_votes_data) @@ -458,6 +495,14 @@ def find_discrepancies( # surface it for review without gating accept-all. if current_key == "role" and path.startswith("ligand_copies["): record["gating"] = False + # Same site_ref carve-out as the differing-value branch: an apo + # placeholder or a shipped 'unknown' cannot encode a wrong site, + # so a near-tie there is advisory; a near-tie between two real + # sites keeps gating. + if current_key == "site_ref" and _site_ref_controversy_is_non_gating( + path, best_run_data + ): + record["gating"] = False discrepancies.append(record) return discrepancies @@ -527,15 +572,22 @@ def _record(node_path: str, node: Any) -> dict[str, Any]: confidence = copy_row.get("confidence") for field in ("site_ref", "role"): value = copy_row.get(field) - flags.append( - { - "path": f"ligand_copies[{key}].{field}", - "best_run_value": value, - "majority_vote_value": value, - "all_votes": {}, - "needs_review": True, - "low_confidence": confidence, - } - ) + flag: dict[str, Any] = { + "path": f"ligand_copies[{key}].{field}", + "best_run_value": value, + "majority_vote_value": value, + "all_votes": {}, + "needs_review": True, + "low_confidence": confidence, + } + # A per-copy ROLE cannot encode a shipped error: the CSV Role + # column is taken from the compound-level ligand, never a + # ligand_copies row (mirrors the near-tie carve-out in + # find_discrepancies). It stays visible for review but is + # advisory. The per-copy site_ref does drive residue partitioning, + # so it keeps gating here. + if field == "role": + flag["gating"] = False + flags.append(flag) return flags diff --git a/src/gpcr_tools/config.py b/src/gpcr_tools/config.py index 959dc77..7ab2c8b 100644 --- a/src/gpcr_tools/config.py +++ b/src/gpcr_tools/config.py @@ -1439,6 +1439,15 @@ def gpcrdb_aux_type_for(comp_id: str | None) -> str: # and are still surfaced by the SUSPICIOUS_7TM alert). GPCR_MIN_ANNOTATED_TM: int = 4 +# A chain whose every annotated TM helix is resolved counts as COMPLETE even when +# fewer than six TMs were annotated, provided the annotation is substantial (at +# least this many TMs). This covers receptors whose UniProt/RCSB mapping only +# resolved five of the canonical seven helices: with resolved_tms == total_tms +# no mapped TM is unmodeled, so the "incompleteness" is a mapping artifact, not +# missing density. Kept above single-/few-pass partner slugs (total_tms small) +# so those never wave through. +TM_MIN_RECEPTOR_ANNOTATED: int = 5 + TM_ENTITY_FEATURE_TYPES: frozenset[str] = frozenset( { "TRANSMEMBRANE", @@ -1503,6 +1512,132 @@ def gpcrdb_aux_type_for(comp_id: str | None) -> str: "rarr2", "a0a", "mtor", + # Non-receptor chains that carry a GPCRdb entry-name slug but are not 7TM + # receptors: peptide / protein agonists, chemokines, toxins, crystallization + # and expression partners, and signalling-pathway proteins. Without a + # transmembrane span they trip the SUSPICIOUS_7TM tripwire; denylisting their + # slug prefixes keeps them out of the receptor roster. Every prefix below was + # checked against the full GPCRdb real-receptor slug universe (all species) + # and filters zero real receptors. A trailing underscore is the safe form for + # a ligand stem that would otherwise be a prefix of its own receptor family + # (e.g. "npy_" matches the ligand npy_* but not the npy1r_* receptors). + # Adhesion receptors and the V2 receptor slug are deliberately NOT listed: + # their stems cannot be disambiguated from real receptors, so they remain + # covered by the SUSPICIOUS_7TM alert. + # Chemokine ligands + "ccl2", + "ccl5", + "ccl7", + "ccl15", + "ccl19", + "cxcl2", + "cxcl3", + "cxcl5", + "cxcl6", + "cxcl9", + "cxl10", + "cxl11", + "sdf1", + "il8", + "groa", + "x3cl1", + "xcl1", + # Peptide / protein-hormone ligands + "edn1", + "edn3", + "pthy", + "pthr", + "npy_", + "pyy", + "cckn", + "tkn1", + "tknk", + "sms", + "paca", + "gala", + "apel", + "angt", + "ucn1", + "kng1", + "penk", + "pdyn", + "pnoc", + "prrp", + "calca", + "calc_", + "adml", + "adm2", + "secr", + "gast", + "ghrl", + "nmu_", + "nmb_", + "nms", + "npff_", + "vip_", + "crh_", + "gip_", + "grp_", + "insl5", + "moti", + "orex", + "kiss1", + "cort", + "mch_", + "tip39", + "spxn", + "hunin", + "ela", + "slib", + "ndp", + "rspo1", + "rspo2", + "gp15l", + "paho", + "exe3", + "exe4", + # Toxin / venom peptides + "3si1a", + "3sim3", + "3sim7", + "srtx", + "maxa", + "crfa2", + # Non-receptor proteins (fusion tags, signalling-pathway, viral, other) + "thio", + "coli", + "luci", + "spg1", + "spa", + "gpa1", + "mfal1", + "hema", + "env", + "vmi2", + "da2d", + "cd4", + "co3", + "co5", + "neu1", + "neu2", + "neut", + "gnb5", + "rgs7", + "pp2ba", + "vgf", + "ox26", + "dvl2", + # Accession-named non-receptor chains + "b5xgr7", + "b5z8h1", + "f5gzd9", + "f6vl43", + "o87916", + "q5fwy2", + "q70145", + "q7z3y4", + "q92163", + "v9hw68", ) # --------------------------------------------------------------------------- @@ -1621,6 +1756,14 @@ def gpcrdb_aux_type_for(comp_id: str | None) -> str: ALERT_PREFIX_ALGO_WARNING: str = "[ALGO WARNING]" ALERT_PREFIX_API_UNAVAILABLE: str = "[API_UNAVAILABLE]" ALERT_PREFIX_CHIMERIC_REVIEW: str = "[CHIMERIC G PROTEIN]" +# The alpha5 helix resolves only the coupling FAMILY; members with identical +# alpha5 (e.g. gnai1/gnai2, gnaq/gna11, the transducins) cannot be split from +# structure. When the family is verified and the record is otherwise a native, +# family-consistent G protein this is advisory, not a chimera to resolve. +ALERT_PREFIX_GALPHA_SUBTYPE_UNRESOLVED: str = "[G-ALPHA SUBTYPE UNRESOLVED]" +# The alpha5 family matches but the candidate slugs are non-human orthologs, so +# the species / GPCRdb mapping is unconfirmed -- gating. +ALERT_PREFIX_GALPHA_SPECIES_UNVERIFIED: str = "[G-ALPHA SPECIES UNVERIFIED]" ALERT_PREFIX_MISSED_POLYMER: str = "[UNANNOTATED CHAIN]" ALERT_PREFIX_FUSION_NOTE: str = "[CRYSTALLIZATION FUSION]" ALERT_PREFIX_BINDER_RENAME: str = "[BINDER NAME CORRECTED]" diff --git a/src/gpcr_tools/validator/gating.py b/src/gpcr_tools/validator/gating.py index 29cc1ab..dc69ae0 100644 --- a/src/gpcr_tools/validator/gating.py +++ b/src/gpcr_tools/validator/gating.py @@ -14,6 +14,8 @@ from __future__ import annotations +import re + from gpcr_tools.config import ( ALERT_HALLUCINATION, ALERT_MISSED_PROTOMER, @@ -42,6 +44,43 @@ } ) +# Block key of the per-copy ligand table (one row per modelled copy, each +# recording the binding site that copy occupies). +_LIGAND_COPIES_BLOCK: str = "ligand_copies" + +# The compound-level routing anchor a multi-copy ligand alert is built with, +# e.g. "at 'ligands[CLR]':". Matched so the mirror can re-anchor the same text at +# the per-copy table while keeping the component id visible to the curator. +_LIGANDS_ANCHOR_RE = re.compile(r"at '\.?ligands\[(?P[^\]]*)\]'\s*:?\s*") + + +def _mirror_at_ligand_copies(alert_type: str, message: str | None) -> str: + """Re-anchor a compound-level ligand alert at the per-copy ligand table. + + A multi-copy ligand alert is raised against the compound (``ligands[]``), + but the compound-level row cannot say which copy sits at which site -- only the + per-copy table can, and that is where a wrong site assignment is corrected. + Rewriting the anchor to ``ligand_copies`` makes the alert reachable from that + table, and the component id is carried over in words so the curator still knows + which compound is meant without re-introducing a compound-level anchor (which + would route the mirror straight back to the compound table). + + The ``[TYPE]`` label is preserved exactly once via :func:`ensure_alert_prefix`, + so a back-catalogue message stored without its label still gets one, and a + current message keeps the single label it already carries. A message with no + recognizable compound anchor is simply anchored at the per-copy table as-is. + """ + label = f"[{alert_type}]" + body = ensure_alert_prefix(alert_type, message)[len(label) :].strip() + + anchor = f"at '{_LIGAND_COPIES_BLOCK}':" + match = _LIGANDS_ANCHOR_RE.search(body) + if match: + anchor = f"at '{_LIGAND_COPIES_BLOCK}' (component {match.group('component')}):" + body = (body[: match.start()] + body[match.end() :]).strip() + + return f"{label} {anchor} {body}".strip() + def oligomer_gating_warnings(oligo: dict | None) -> list[str]: """Oligomer findings that should gate review, as curator-facing strings. @@ -50,6 +89,11 @@ def oligomer_gating_warnings(oligo: dict | None) -> list[str]: surfaces under ``receptor_info`` (a corrected chain id, a gating oligomer alert, a multi-copy ligand, an incomplete 7TM domain). Returns ``[]`` for an absent or empty oligomer analysis. + + A gating multi-copy ligand alert yields TWO strings: the compound-anchored + original plus a copy re-anchored at ``ligand_copies``, because the per-copy + table is the only block whose fields can record which copy sits at which + binding site (see :func:`_mirror_at_ligand_copies`). """ if not oligo: return [] @@ -67,6 +111,18 @@ def oligomer_gating_warnings(oligo: dict | None) -> list[str]: for alert in oligo.get("alerts") or []: atype = alert.get("type") or "" if atype in _GATING_OLIGOMER_ALERTS: + if atype == ALERT_OLIGOMER_DISAGREEMENT and not alert.get("gating", True): + # An OD alert the aggregator downgraded to advisory: the AI released + # monomer while the classifier counted >=2 same-slug chains, but + # RCSB's own global biological assembly records the receptor as a + # single copy (a Monomer assembly, or an all-single stoichiometry), + # so the released monomer agrees with both the AI and RCSB. Still + # surfaced to the curator via the alert list, but not gating -- the + # same non-gating policy the parallel ASSEMBLY_MISMATCH advisory + # already carries. The flag defaults to True so a back-catalogue OD + # alert recorded before the flag existed still gates rather than + # being silently waved through. + continue # The "at 'receptor_info'" prefix is a routing anchor so this alert # buckets under the receptor block during review. ensure_alert_prefix # keeps the message's own "[TYPE]" label present exactly once -- @@ -74,10 +130,23 @@ def oligomer_gating_warnings(oligo: dict | None) -> list[str]: # duplicate it), while older recorded data needs it added. message = ensure_alert_prefix(atype, alert.get("message")) warnings.append(f"OLIGOMER ALERT at 'receptor_info': {message}") - elif atype == ALERT_MULTI_COPY_LIGAND: + elif atype == ALERT_MULTI_COPY_LIGAND and alert.get("gating", True): + # A multi-copy ligand gates only when its copies sit at distinct binding + # sites; the aggregator stamps that decision on the alert's ``gating`` + # flag (copies sharing one site are advisory, still surfaced elsewhere). + # The flag defaults to True so a back-catalogue alert recorded before the + # flag existed still gates rather than being silently waved through. # Already carries its own 'ligands[...]' path, so it buckets with the # ligand block during review rather than under receptor_info. warnings.append(alert.get("message") or "") + # ... but the compound-level ligand row has no field for a per-copy + # site: the copies are only separable in the per-copy ligand table, so + # that is the one block where a curator can act on this alert. Emit an + # additional copy of the same finding re-anchored there, so the per-copy + # table is opened for review instead of passing through unseen. Only a + # gating alert is mirrored -- copies that share one binding site need no + # per-copy decision, so mirroring those would be pure prompt noise. + warnings.append(_mirror_at_ligand_copies(atype, alert.get("message"))) if any(c.get("7tm_status") == TM_STATUS_INCOMPLETE for c in oligo.get("all_gpcr_chains") or []): warnings.append( diff --git a/src/gpcr_tools/validator/ligand_validator.py b/src/gpcr_tools/validator/ligand_validator.py index c4312fd..cd44da3 100644 --- a/src/gpcr_tools/validator/ligand_validator.py +++ b/src/gpcr_tools/validator/ligand_validator.py @@ -31,6 +31,7 @@ VALIDATION_MATCHED_POLYMER, VALIDATION_MATCHED_SMALL_MOLECULE, VALIDATION_SKIPPED_APO, + ligand_routed_to_aux, ) from gpcr_tools.validator.api_clients import SynonymCache, check_pubchem_synonym_match from gpcr_tools.validator.chimera import ( @@ -99,12 +100,13 @@ def validate_and_enrich_ligands( enriched_entry: dict[str, Any], *, synonym_cache: SynonymCache | None = None, + advisory_notes: list[str] | None = None, ) -> list[str]: """Validate AI-reported ligands and inject chemical identifiers. Mutates *best_run_data* ligand dicts in-place. - Returns a list of warning strings (``GHOST_LIGAND`` detections, plus apo - placeholders that coexist with real ligands). + Returns a list of gating warning strings (``GHOST_LIGAND`` detections, plus + apo placeholders that coexist with a real, functional ligand). When *synonym_cache* is provided, a ligand that carries a model-supplied PubChem CID but matched no chemical component (no authoritative CID to copy) @@ -112,10 +114,20 @@ def validate_and_enrich_ligands( different molecule is blanked and flagged. Without a cache this step is skipped and the pass remains fully offline. + Some findings are advisory rather than gating: a PubChem CID that was already + blanked, and an apo placeholder whose only companions are structural + cofactors / ions / lipids. When *advisory_notes* is supplied those messages + are routed to it (the caller records them as detector notes, not gating + warnings); when it is omitted they fall back into the returned list so no + finding is lost for callers that do not separate the two channels. + Warning format: ``f"GHOST_LIGAND at 'ligands[{label}]': '{name}' ({cid}) not found in API entities."`` """ warnings: list[str] = [] + # Advisory findings route here when the caller opts in; otherwise they merge + # into the returned warnings list, preserving the single-channel behaviour. + advisory = advisory_notes if advisory_notes is not None else warnings ligands = best_run_data.get("ligands") if not isinstance(ligands, list) or not ligands: return warnings @@ -187,8 +199,8 @@ def validate_and_enrich_ligands( ) if synonym_cache is not None: - _gate_keyless_pubchem_ids(ligands, synonym_cache, warnings) - _warn_on_apo_with_real_ligands(ligands, warnings) + _gate_keyless_pubchem_ids(ligands, synonym_cache, warnings, advisory) + _warn_on_apo_with_real_ligands(ligands, warnings, advisory) _warn_on_role_site_mismatch(ligands, warnings) _warn_on_g_protein_peptide_as_ligand(ligands, api["poly_by_chain"], warnings) _warn_on_multiple_agonists(ligands, warnings) @@ -199,6 +211,7 @@ def _gate_keyless_pubchem_ids( ligands: list[Any], synonym_cache: SynonymCache, warnings: list[str], + advisory: list[str], ) -> None: """Cross-check a model-supplied PubChem CID against the CID's own synonyms. @@ -245,7 +258,11 @@ def _gate_keyless_pubchem_ids( if verdict is False: lig["pubchem_id"] = None display = name or lig.get("chem_comp_id") or "?" - warnings.append( + # The wrong CID is already blanked above, so the shipped record is + # correct. Gating on the fact that a now-removed CID was once wrong + # gates already-corrected data, so this is advisory: the curator sees + # what was cleared without the PDB being held for review over it. + advisory.append( f"PubChem CID Mismatch at 'ligands': CID '{cid}' is not a known " f"synonym of '{display}' -- the identifier appears to name a " f"different compound and has been cleared." @@ -377,11 +394,21 @@ def _warn_on_g_protein_peptide_as_ligand( ) -def _warn_on_apo_with_real_ligands(ligands: list[Any], warnings: list[str]) -> None: +def _warn_on_apo_with_real_ligands( + ligands: list[Any], warnings: list[str], advisory: list[str] +) -> None: """Flag (for the curator) an apo placeholder sitting alongside real ligands — a contradiction worth a human's eye. Emits a warning only; the data is left untouched so the curator decides what is correct. A buffer/solvent next to an apo entry is normal and does not warn. + + Severity is role-aware. An apo placeholder next to nothing but structural + cofactors / ions / lipids (every coexisting molecule routes to the auxiliary + catalogue rather than ligands.csv) is a benign, common configuration, so the + finding is advisory. But if any coexisting molecule is a functional / + allosteric ligand, a ghost the validator could not place, or carries an + unknown / blank role -- anything that classifies as a real ligand row -- the + apo/ligand contradiction is genuine and stays gating. """ real_statuses = { VALIDATION_MATCHED_SMALL_MOLECULE, @@ -399,11 +426,17 @@ def _warn_on_apo_with_real_ligands(ligands: list[Any], warnings: list[str]) -> N ] if has_apo and real: names = ", ".join(str(lig.get("name") or lig.get("chem_comp_id") or "?") for lig in real) - warnings.append( + message = ( f"APO_WITH_LIGANDS at 'ligands': an apo (ligand-free) placeholder " f"coexists with {len(real)} real ligand(s) [{names}] — verify whether " f"this structure is truly apo." ) + # Downgrade to advisory ONLY when every coexisting molecule is a structural + # cofactor / ion / lipid (routes to the auxiliary catalogue). A ghost or a + # functional/allosteric/unknown-role molecule classifies as a real ligand + # row and keeps the finding gating. + exclusively_structural = all(ligand_routed_to_aux(lig) for lig in real) + (advisory if exclusively_structural else warnings).append(message) def _warn_on_multiple_agonists(ligands: list[Any], warnings: list[str]) -> None: diff --git a/src/gpcr_tools/validator/oligomer.py b/src/gpcr_tools/validator/oligomer.py index 96aa4d3..447db06 100644 --- a/src/gpcr_tools/validator/oligomer.py +++ b/src/gpcr_tools/validator/oligomer.py @@ -54,6 +54,7 @@ OLIGOMER_NO_GPCR, TM_COVERAGE_THRESHOLD, TM_ENTITY_FEATURE_TYPES, + TM_MIN_RECEPTOR_ANNOTATED, TM_STATUS_COMPLETE, TM_STATUS_INCOMPLETE, TM_STATUS_UNKNOWN, @@ -221,7 +222,18 @@ def _analyze_tm_for_entity_instance( resolved_tms += 1 total_tms = len(tm_regions) - status = TM_STATUS_COMPLETE if resolved_tms >= 6 else TM_STATUS_INCOMPLETE + # A chain is COMPLETE when at least six TM helices are resolved, OR when every + # annotated TM is resolved and the annotation is substantial (>= five helices). + # The second clause rescues receptors whose UniProt/RCSB mapping only exposed + # five of the canonical seven TMs: resolved_tms == total_tms means no mapped + # TM is unmodeled, so the shortfall is a mapping artifact rather than missing + # density. Chains with any unmodeled TM (resolved_tms < total_tms) or a small + # annotation (single-/few-pass partner slugs) stay INCOMPLETE. + fully_resolved_receptor = total_tms >= TM_MIN_RECEPTOR_ANNOTATED and resolved_tms == total_tms + if resolved_tms >= 6 or fully_resolved_receptor: + status = TM_STATUS_COMPLETE + else: + status = TM_STATUS_INCOMPLETE return {"resolved_tms": resolved_tms, "total_tms": total_tms, "status": status} @@ -942,6 +954,27 @@ def _get_assembly_cross_check( for s in symmetry_blocks ) + # A second, wider scan across EVERY candidate assembly, not just the chosen + # one. An entry can deposit several candidate assemblies that disagree -- an + # author-defined monomer alongside a software-predicted homo-dimer. The + # chosen-assembly flag above answers "does the assembly we display record a + # homo-oligomer"; this one answers "does ANY deposited assembly record one", + # which is the right question before waiving a receptor-count disagreement: + # RCSB contradicting itself is not corroboration. Kept as a separate key so + # ``has_homo_symmetry`` (and the ASSEMBLY_MISMATCH advisory that reads it) + # keeps its established meaning. + homo_symmetry_in_any_candidate = ( + any( + isinstance(sym.get("oligomeric_state"), str) + and sym["oligomeric_state"].strip().lower().startswith("homo") + for asm in assemblies + if (asm.get("pdbx_struct_assembly") or {}).get("rcsb_candidate_assembly") == "Y" + for sym in (asm.get("rcsb_struct_symmetry") or []) + if isinstance(sym, dict) + ) + or has_homo_symmetry + ) + first = symmetry_blocks[0] return { "oligomeric_state": first.get("oligomeric_state"), @@ -949,6 +982,7 @@ def _get_assembly_cross_check( "kind": first.get("kind"), "type": first.get("type"), "has_homo_symmetry": has_homo_symmetry, + "homo_symmetry_in_any_candidate": homo_symmetry_in_any_candidate, "rcsb_candidate_assembly": (chosen.get("pdbx_struct_assembly") or {}).get( "rcsb_candidate_assembly" ), @@ -985,6 +1019,114 @@ def _parse_oligomeric_count(oligomeric_state: Any) -> int | None: return None +_STOICH_COPY_RE = re.compile(r"(\d+)\s*$") + + +def _stoich_copy_count(entry: Any) -> int | None: + """Parse the copy count from one RCSB stoichiometry entry (e.g. ``"A2"`` -> 2). + + RCSB records a symmetry block's stoichiometry as a list of ``