Skip to content

Claims are unconstructed dicts with a silently-defaulting tier — add ExtendedClassificationResult.add_claim #150

Description

@NoopDog

The defect

The codebase has two record types in the classification result. One is guarded by a constructor with required, checked slots. The other is a bare dict literal with an implicit schema and a silently-defaulting required field.

Field entry{value, status, evidence} — is locked down. models.build_field_entry is "the single place that assembles the output shape"; _assert_coherent raises if value and status contradict; set_field raises on an unknown field or an invalid status. A sentinel cannot reach the value slot.

Claim — the evidence dict appended to field_evidence[fld] — has no constructor. Its schema exists only in the consumers that read it:

key read by if omitted
value / status _claim_declaration (rule_engine.py:186) claim silently ignored during resolution
tier evaluate_claims (rule_engine.py:263-264), via .get("tier", 0) silently becomes tier 0
rule_id rule_engine.py:138,151, via e["rule_id"] KeyError, loudly
reason display only nothing

The failure modes are inconsistent, and the one that silently changes the answer — tier — is the one with a defensive default.

Why it doesn't bite in the rules path

_apply_rule (rule_engine.py:583) builds every rule-derived claim from a single evidence_entry literal carrying rule_id, reason, tier, then spreads it into a value-claim or a status-claim. One construction site. Tier is never forgotten, and the loader already enforces value-xor-status per field (rule_loader.py:349 raises when a field appears in both then and then.status).

_apply_rule is already the API this issue asks for. It just isn't exposed.

Where it does bite

Content classifiers hand-roll claims outside that constructor. In header_classifier.py there are 19 claim literals, of which exactly 1 sets a tier (added in #148/#151). rule_engine.py:672 (infer_assay_type) omits it too.

Worse, those sites maintain two independent sources of truth — the claim list and the field's value — and nothing checks they agree:

result.field_evidence["data_type"].append({... "value": "pangenome.reference" ...})
result.set_field("data_type", "pangenome.reference")   # nothing verifies these match

This is not hypothetical. In #151 the first draft omitted tier from the appended rGFA claim. evaluate_claims would have defaulted it to 0, lost to the tier-1 pangenome claim via higher_specificity_override, and resolved to pangenome — contradicting the pangenome.reference written by set_field. It only stayed hidden because nothing re-runs resolution on that path. The evidence would have contradicted the answer, silently.

Proposed API

One method on ExtendedClassificationResult:

def add_claim(self, fld, *, rule_id, reason, tier, value=None, status=None):
    """Append a claim and re-resolve the field from all its claims."""
    self._require_field(fld)
    # value xor status — the same invariant rule_loader enforces for rules
    claim = {"rule_id": rule_id, "reason": reason, "tier": tier}
    claim["value" if value is not None else "status"] = value or status
    self.field_evidence[fld].append(claim)
    ev = evaluate_claims(self.field_evidence[fld])
    self.set_field(fld, ev["value"], ev["status"])

What this buys:

  1. tier cannot be forgotten — required keyword slot.
  2. Value cannot diverge from evidence — it is derived from the claim list, not written alongside it. One source of truth.
  3. Conflict detection everywhere, freeevaluate_claims runs on every addition, so a competing same-tier claim raises a conflict instead of being silently overwritten by whichever line runs last.
  4. evaluate_claims can stop being defensive.get("tier", 0) becomes claim["tier"], turning a silent wrong answer into a loud KeyError.
  5. _apply_rule collapses onto it, so rules and content classifiers share one path.

Migration

18 wholesale field_evidence[...] = [...] assignments in header_classifier.py (BAM contig detection, VCF, FASTQ, FASTA, BED) each discard the tier-1/2 claims the engine produced and replace them with a single untiered entry. Each becomes an add_claim call.

rule_engine.py:668 is a different smell — it filters a stale not_classified placeholder out of the list rather than clobbering it. Worth revisiting once add_claim re-resolves on every addition, since the placeholder would no longer need removing by hand.

The one already-correct site is header_classifier.py:271 (extend), plus the engine's own appends.

Notes on scope

  • FASTA's logic (contig counting, >= 20 matching chromosomes to call a reference, assembler/transcript pattern buckets) will not reduce to a declarative YAML when: predicate. That is fine and is the point: add_claim lets imperative classifiers emit claims without becoming rules. A gfa_header YAML scope is optional and secondary.
  • Migrating one classifier at a time is safe — add_claim and the existing assignment can coexist during the transition. But the golden fixture (tests/fixtures/golden/expected_output.json) pins evidence for every file type, so each migration will show its provenance change there. That is the review signal, not a problem.
  • Expect real output changes: fields that currently show one hand-rolled entry will show the full derivation chain. Detect pangenome reference graphs from rGFA content (#148) #151 already did this for GFA — data_type evidence went from [rgfa_stable_rank_reference] to [tier=1 pangenome_graph, tier=3 rgfa_stable_rank_reference], matching what the engine emits for the -mc- case.

Tasks

  • Add ExtendedClassificationResult.add_claim with required rule_id/reason/tier and value-xor-status enforcement.
  • Re-express _apply_rule in terms of it (or share the claim constructor).
  • Migrate the 18 header_classifier.py sites, one classifier per commit, regenerating the golden each time.
  • Add tier to rule_engine.py:672 (infer_assay_type).
  • Tighten evaluate_claims to claim["tier"] once no caller can omit it.
  • Revisit the rule_engine.py:668 placeholder filter.

Refs: #148/#151 (GFA content check — the near-miss that surfaced this, and the one site already appending with a tier), #116/#136 (status/value split, which fixed the field entry half of this problem), #56 (claim/evidence model).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    Status
    No status

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions