Skip to content

test(corpus): shared adversarial trap corpus with disposition scoring - #472

Merged
kevincostner17 merged 4 commits into
mainfrom
feat/trap-corpus
Sep 16, 2026
Merged

kevincostner17 merged 4 commits into
mainfrom
feat/trap-corpus

Conversation

@kevincostner17

Copy link
Copy Markdown
Contributor

Why

Four gold corpora already exist under benchmarks/ — Gauntlet, TruthBench, CleanBench and the fixtures generator — but none is reachable as a fixture from the ~272 ordinary files under tests/. Every test re-invents its own trap literals, which is why '1,000', '1.000', '0001' and 'None' appear as literals in zero test files today.

What this adds

benchmarks/corpus/ — one importable corpus (pythonpath = ["."] already makes benchmarks.* importable from tests/). It does not replace the existing corpora: adapters.py re-exports Gauntlet's 91 and TruthBench's 1536 cases through the same TrapCase type, so all three are scored with one vocabulary.

The organising idea is the one the library's design turns on: the same token means different things in different fields. Cases are keyed by (token, role) and the corpus deliberately carries the same token under several roles:

token role expected
007 customer_id / postal_code preserve
007 quantity repair → 7
M gender / shirt_size / marital_status repair → Male / Medium / Married
apple company_name preserve
apple amount / ticker quarantine

A cleaner keyed on the value alone passes every single-role test and fails here.

Six dispositions, back-compatibly

gauntlet/metrics.REVIEW_ACTIONS folds quarantine, manual_review and reject together, so no gold corpus could say "this must be rejected, not merely queued for a human". QUARANTINE and REJECT are now first-class. satisfies() widens a plain REVIEW label to any review-family outcome — exactly what REVIEW_ACTIONS means today — so the existing four-value fixtures keep their meaning and keep passing.

Specification gaps are recorded, not invented

Where the repository genuinely does not define an expectation, the case carries expected=None plus a spec_gap naming the missing decision, and is measured rather than asserted. Twelve are captured, including the undeclared-locale pair 1,200/1.200 and the undeclared-dayfirst pair 01/02/2026/02/01/2026, which must resolve under the same convention or ordering silently inverts.

Two cases were corrected during review: I initially asserted PRESERVE for NA/None, then found the repo had already decided otherwise on purpose — Gauntlet injects NA as family sentinel_collision and calls nulling None "contract behaviour". Asserting a preference over a documented decision would be inventing contract.

Scoring

scoring.py derives the observed disposition from what actually happened to the cell, never from what the report claims, and reports the metric set as rates. A wrong repair counts as a corruption, not a repair, so a confident mistake cannot hide inside a success metric.

The two committed assertions avoid thresholds deliberately — freezing today's numbers would make current behaviour the specification. One guards the metric names; the other asserts audit completeness stays at 1.0, i.e. if the library changes a cell, the report names it. That currently holds across the whole corpus.

Result

Running this harness is what surfaced the currency locale defect in #471.

Verification

  • tests/test_trap_corpus.py: 20 tests, including a contract test that every fieldcheck.ACTIONS value maps to a disposition, so an action added to the library fails loudly here instead of being mis-scored.
  • No library behaviour is changed by this PR.

Four gold corpora already exist under benchmarks/ (Gauntlet, TruthBench,
CleanBench and the fixtures generator), but none is reachable as a fixture
from the ~272 ordinary files under tests/. Each test therefore re-invents its
own trap literals, which is why '1,000', '1.000', '0001' and 'None' appear as
literals in zero test files today.

benchmarks/corpus/ adds one importable corpus (pythonpath = ["."] already
makes benchmarks.* importable from tests/). It does not replace the existing
corpora: adapters.py re-exports Gauntlet's 91 and TruthBench's 1536 cases
through the same TrapCase type, so all three are scored with one vocabulary.

The organising idea is the one the library's design turns on: the same token
means different things in different fields. Cases are keyed by (token, role)
and the corpus deliberately carries the same token under several roles with
different expected dispositions -- '007' preserves as an identifier and
repairs to 7 as a quantity; 'M' expands to Male, Medium or Married depending
on the column; 'apple' is a valid company name and an unparseable amount. A
cleaner keyed on the value alone passes every single-role test and fails here.

dispositions.py adds QUARANTINE and REJECT to the four values the existing
harnesses share. gauntlet/metrics.REVIEW_ACTIONS currently folds quarantine,
manual_review and reject together, so no gold corpus could say "this must be
rejected, not merely queued". Back-compat is preserved: satisfies() widens a
plain REVIEW label to any review-family outcome, exactly as REVIEW_ACTIONS
does, so existing four-value fixtures keep their meaning.

Where the repository genuinely does not define an expectation, the case
records expected=None plus a spec_gap naming the missing decision, and is
measured rather than asserted. Twelve such gaps are captured, including the
undeclared-locale pair '1,200'/'1.200' and the undeclared-dayfirst pair
'01/02/2026'/'02/01/2026', which must resolve under the same convention or
ordering silently inverts.

test_trap_corpus.py guards the corpus itself, including a contract test that
every fieldcheck.ACTIONS value maps to a disposition, so an action added to
the library fails loudly here instead of being mis-scored.
Three cases asserted PRESERVE for 'NA' and 'None' on the reasoning that
Namibia, the surname Na and the brand None are real data. Running them against
the library showed the repository has already decided otherwise, deliberately:
Gauntlet injects 'NA' into a country column as family 'sentinel_collision'
(fixtures.py:311) and labels 'None' in a company column repair-to-missing with
the note "a documented sentinel; nulling it is contract behaviour"
(fixtures.py:178). Asserting a preference over a decision the repo has made and
labelled would be inventing contract, which the corpus exists to avoid.

'None'/product_name is therefore encoded as REPAIR, matching the gold label.

'NA'/country and 'NA'/last_name become specification gaps, because here the
repository genuinely contradicts itself rather than merely deciding something
unpalatable: fieldcheck.py:466 states "explicit vocabulary outranks generic
null markers -- NA may be Namibia" and validate_fields honours a declared
allowed_values, while fd.clean's normalize_sentinels ignores allowed_values and
nulls the value anyway. The same declared fact is respected by one public API
and ignored by another, so the expected disposition is undefined until the two
layers agree.

Recorded as FD2-001 with the full route table; no library behaviour is changed
by this commit.
frames.py builds a realistic frame per case: eight plausible values for the
role plus the trap in the last row. The filler matters twice over -- the
semantic layer ignores a column with fewer than MIN_DISTINCT_SUPPORT (5)
distinct values, and without it "nothing changed" could mean "the library gave
up" rather than "the value was preserved". A second anchor column keeps the row
alive so a nulled cell is not confused with a dropped row.

scoring.py derives the observed disposition from what actually happened to the
cell, never from what the report claims, and reports the Phase 23 metric set as
rates: repair precision/recall, false positive/negative rate, preservation,
review, corruption, escape and audit completeness. A wrong repair is counted as
a corruption rather than folded into the repair count, so a confident mistake
cannot hide inside a success metric. Cases whose expectation is a recorded
specification gap are measured and reported but never scored pass or fail.

The two committed assertions deliberately avoid thresholds -- freezing today's
numbers would make current behaviour the specification. One guards the metric
names, the other asserts audit completeness stays at 1.0, which is the property
that matters most here: if the library changes a cell, the report names it.
That currently holds across the whole corpus.

Running this harness is what surfaced the currency locale defect.
@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: ab8a7748-9cf4-413f-b793-fa98bde8d287


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

FreshData benchmark report — performance

  • freshdata: ?
  • python: ?
  • platform: ?
fixture n_rows n_cols p50 s p95 s peak MB repair % false-repair % preserve % trust monotonic export %

Authored-code reduction (Metric 6)

…ports

Two fixes found by running `ruff check .` and the full suite the way CI does,
rather than only the paths I had touched.

- Function-level imports in the two scorer tests tripped PLC0415, which failed
  quality-fast on #472. Hoisted.

- The scorer passed every case's semantic_type straight to validate_fields,
  which warned 42 times that nothing would be validated. That is the library
  telling the truth: SEMANTIC_TYPES (17 terms, what fd.infer_roles returns and
  semantic_context accepts) and fieldcheck._KNOWN_SEMANTIC_TYPES (23 terms,
  what FieldSpec validates) share only 10 of 30 terms. address, boolean_like,
  category_code, national_id, postal_code and quantity_with_unit are real
  semantic types that validate_fields does not know, so a user who feeds an
  inferred "postal_code" back into a FieldSpec is told nothing will be checked.

  The scorer now only declares a type that fieldcheck knows, and a new test
  pins both difference sets so that closing or widening the gap is a
  deliberate, visible change rather than drift. The test asserts what the split
  *is*, not that it is correct; recorded as FD2-003 for a naming decision.

  Credit where due: the warning names the column, the unknown type, the
  consequence and the known types, so this is a coherence problem rather than
  a silent one.
@kevincostner17
kevincostner17 merged commit 2dbd4bd into main Sep 16, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants