Skip to content

Commit 1953d43

Browse files
fix(rewrite): close record-drop gaps flagged in review
Two small-model robustness gaps surfaced by Greptile on the disposition path: - _pessimistic_fallback_disposition could still drop a row: with empty entity context it built an empty SensitivityDispositionSchema, which raises on the min_length=1 tripwire (and again in the downstream parser), and both call-sites in _reconstruct_full_disposition_column were unguarded. It now emits a single no-op (leave_as_is/low) disposition in that pipeline-invariant case, logging loudly — the row survives and the no-op never reaches the rewrite (excluded from protected_entities). Also removes the dead duplicate return branch. - SimpleDispositionItem._coerce_scalar_to_str now coerces unexpected container types (list/dict) to "" instead of returning them unchanged, so one drifted field no longer fails the whole item (which would discard every disposition for the row and force a pessimistic fallback). Adds regression tests for the empty-context no-op guarantee (function + column level) and container-value coercion.
1 parent a6017ab commit 1953d43

4 files changed

Lines changed: 72 additions & 6 deletions

File tree

src/anonymizer/engine/rewrite/sensitivity_disposition.py

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -335,11 +335,32 @@ def _pessimistic_fallback_disposition(
335335
)
336336
)
337337
if not items:
338-
# Genuinely no entities at all in context — the orchestrator should
339-
# have short-circuited before this step. Still better to raise here
340-
# than to silently emit garbage; SensitivityDispositionSchema's
341-
# min_length=1 invariant will surface the bug.
342-
return SensitivityDispositionSchema(sensitivity_disposition=items)
338+
# Genuinely no entities at all in context. The orchestrator should have
339+
# short-circuited rows with no detected entities before this step, so
340+
# this is a pipeline-invariant violation — but this is the last-resort
341+
# path whose contract is "never drop the row." Emitting an empty list
342+
# would raise on SensitivityDispositionSchema's (and the downstream
343+
# parser's) min_length=1 invariant and drop the record, so we log loudly
344+
# and emit a single no-op (leave_as_is/low) disposition instead. It is
345+
# excluded from protected_entities, so it never reaches the rewrite.
346+
logger.error(
347+
"pessimistic fallback: empty entity context at the disposition step "
348+
"(orchestrator should have short-circuited entity-free rows); emitting "
349+
"a single no-op disposition so the row is not dropped"
350+
)
351+
items.append(
352+
EntityDispositionSchema(
353+
id=1,
354+
source="tagged",
355+
category="quasi_identifier",
356+
sensitivity="low",
357+
entity_label="",
358+
entity_value="",
359+
protection_method_suggestion="leave_as_is",
360+
combined_risk_level=derive_combined_risk_level("quasi_identifier", "leave_as_is", "low"),
361+
protection_reason=template_protection_reason("quasi_identifier", "leave_as_is", "low"),
362+
)
363+
)
343364
return SensitivityDispositionSchema(sensitivity_disposition=items)
344365

345366

src/anonymizer/engine/schemas/rewrite.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,9 +465,14 @@ class SimpleDispositionItem(BaseModel):
465465
def _coerce_scalar_to_str(cls, v: object) -> str:
466466
if v is None:
467467
return ""
468+
if isinstance(v, str):
469+
return v
468470
if isinstance(v, (int, float, bool)):
469471
return str(v)
470-
return v
472+
# Unexpected container (list/dict) from a drifted response: coerce to ""
473+
# rather than letting pydantic raise on the whole SimpleDispositionItem.
474+
# The reconstructor recovers the true value from trusted entity context.
475+
return ""
471476

472477

473478
class SimpleDispositionResult(BaseModel):

tests/engine/test_disposition_reconstructor.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,24 @@ def test_unmapped_label_falls_back_to_quasi(self) -> None:
345345
item = result.sensitivity_disposition[0]
346346
assert item.category == "quasi_identifier"
347347

348+
def test_empty_context_returns_valid_noop_instead_of_raising(self) -> None:
349+
"""Empty context (a pipeline-invariant violation) must not raise on the
350+
SensitivityDispositionSchema min_length=1 tripwire and drop the row;
351+
the fallback emits a single no-op (leave_as_is/low) disposition."""
352+
result = _pessimistic_fallback_disposition([], [])
353+
assert len(result.sensitivity_disposition) == 1
354+
item = result.sensitivity_disposition[0]
355+
assert item.protection_method_suggestion == "leave_as_is"
356+
assert item.combined_risk_level == "low"
357+
assert result.protected_entities == [] # no-op never reaches the rewrite
358+
359+
def test_all_blank_slots_returns_valid_noop(self) -> None:
360+
"""Slots whose label/value strip to empty are skipped; if that empties
361+
the disposition, the no-op guarantee still holds (no raise/no drop)."""
362+
result = _pessimistic_fallback_disposition([{"value": "", "labels": [""]}], [])
363+
assert len(result.sensitivity_disposition) == 1
364+
assert result.sensitivity_disposition[0].protection_method_suggestion == "leave_as_is"
365+
348366

349367
# ---------------------------------------------------------------------------
350368
# _reconstruct_full_disposition_column (workflow glue)
@@ -394,6 +412,16 @@ def test_empty_simple_falls_back_to_pessimistic(self) -> None:
394412
assert items[0]["entity_label"] == "first_name"
395413
assert items[0]["protection_method_suggestion"] == "replace"
396414

415+
def test_empty_simple_and_empty_context_does_not_drop_row(self) -> None:
416+
"""Both unguarded fallback call-sites: empty simple output AND empty
417+
entity context must still yield a valid row (the column generator must
418+
never raise out and drop the record)."""
419+
row = self._row({"sensitivity_disposition": []}, ebv=[], latent=[])
420+
out = _reconstruct_full_disposition_column(row)
421+
items = out[COL_SENSITIVITY_DISPOSITION]["sensitivity_disposition"]
422+
assert len(items) == 1
423+
assert items[0]["protection_method_suggestion"] == "leave_as_is"
424+
397425
def test_invalid_simple_payload_falls_back(self) -> None:
398426
"""If ``SimpleDispositionResult.model_validate`` raises, fall back
399427
to pessimistic disposition rather than dropping the row."""

tests/engine/test_small_model_drift.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,18 @@ def test_none_in_str_fields_coerces_to_empty(self) -> None:
120120
assert item.entity_label == ""
121121
assert item.category == ""
122122

123+
def test_container_values_in_str_fields_coerce_to_empty(self) -> None:
124+
"""A model emitting a list/dict for a scalar str field must not fail the
125+
whole item (which would discard every disposition for the row); coerce
126+
to "" and let the reconstructor recover from trusted context."""
127+
result = SimpleDispositionResult.model_validate(
128+
[{"id": 1, "entity_label": ["first", "name"], "category": {"x": 1}, "entity_value": "Alice"}]
129+
)
130+
item = result.sensitivity_disposition[0]
131+
assert item.entity_label == ""
132+
assert item.category == ""
133+
assert item.entity_value == "Alice"
134+
123135

124136
# ---------------------------------------------------------------------------
125137
# MeaningUnits — bare list, aspect normalize, importance default, id renumber

0 commit comments

Comments
 (0)