Plan 006: Characterization tests for `EntityExtractor` and the article-processor extraction flow

[strickvl]

Plan 006: Characterization tests for EntityExtractor and the article-processor extraction flow

Executor instructions: Follow this plan step by step. Run every
verification command and confirm the expected result before moving to the
next step. If anything in the "STOP conditions" section occurs, stop and
report — do not improvise. When done, update the status row for this plan
in plans/README.md.

Drift check (run first): git diff --stat 5b5c634..HEAD -- src/engine/extractors.py src/engine/article_processor.py
If either file changed since this plan was written, compare the
"Current state" excerpts against the live code before proceeding; on a
mismatch, treat it as a STOP condition.

Status

• Priority: P2
• Effort: M
• Risk: MED (tests may surface real bugs; that is the point)
• Depends on: none (but Plan 010 depends on THIS)
• Category: tests
• Planned at: commit 5b5c634, 2026-06-12

Why this matters

EntityExtractor (src/engine/extractors.py, 168 lines) and
ArticleProcessor's extraction orchestration
(src/engine/article_processor.py, 483 lines) are the heart of the pipeline
— and no test file imports EntityExtractor at all. The only
article-processor tests (tests/test_extraction_retry.py) cover two small
helper functions (_should_retry_extraction, _build_repair_hint), not the
dispatch, prompt assembly, or retry flow. These are high-churn files (12
and 7 commits in the last 50), and Plan 010 wants to refactor the
neighboring merge module. Characterization tests pin down today's behavior so
refactors and model-provider changes can be verified instead of hoped about.

Current state

• src/engine/extractors.py — EntityExtractor:

  • __init__(entity_type, domain) (line 38) raises
    ValueError(f"Unsupported entity type: {entity_type}") for unknown
    types; valid types come from ENTITY_MODEL_GETTERS (people,
    organizations, locations, events).

  • extract_cloud(text, model=CLOUD_MODEL, temperature=0, repair_hint=None)
    (line 54): gets the dynamic Pydantic model via
    self._model_getter(self.domain), builds
    system_prompt = get_system_prompt(self.entity_type, self.domain), and
    if repair_hint is set appends it:
    system_prompt = system_prompt + "\n\n" + repair_hint. Then calls
    extract_entities_cloud(text=..., system_prompt=..., response_model=List[Entity], model=..., temperature=..., entity_type=...).

  • extract_local(...) (line 85): identical shape, calls
    extract_entities_local(...) with OLLAMA_MODEL default.

  • extract(text, model_type="gemini", model=None, temperature=0)
    (line 119): dispatch —

    if model_type == "ollama":
        return self.extract_local(text=text, model=model or OLLAMA_MODEL, ...)
    else:
        return self.extract_cloud(text=text, model=model or CLOUD_MODEL, ...)

  • Factories at lines 148–167: create_people_extractor(domain) etc.

  • extract_entities_cloud / extract_entities_local /
    get_system_prompt are imported at the top of the module — so patch
    them in the src.engine.extractors namespace.

• src/engine/article_processor.py — ArticleProcessor:
  • extract_single_entity_type(...) (line 153) — runs one entity type's
    extraction, applies QC (run_extraction_qc from
    src.utils.quality_controls), and retries once with a repair hint
    when _should_retry_extraction(flags) is true (severe flags:
    zero_entities, high_drop_rate, many_duplicates,
    many_low_quality_names). Returns a PhaseOutcome
    (src/utils/outcomes.py). Read this method fully before writing tests.
  • extract_all_entities(...) (line 282) — fans out the 4 entity types.
  • check_relevance(...) (line 70) — relevance gate.
• Existing test conventions (follow them):
  • tests/test_extraction_retry.py — class-based tests, plain asserts,
    unittest.mock.patch. Its header comment style: a one-line module
    docstring.
  • Repo memory: patch lazily-imported functions at the source module;
    top-level imports at the importing module. And: entity test dicts need
    all required Pydantic fields — a person needs both name and type,
    e.g. {"name": "Alice Smith", "type": "detainee"}.
• Never call real LLMs in tests; CI has no API keys.

Commands you will need

Purpose	Command	Expected on success
New tests	uv run pytest tests/test_extractors.py tests/test_article_processor_flow.py -v	all pass
Full suite	just test	all pass
Lint+format	just format && just lint	exit 0
Full CI parity	just ci	exit 0

Scope

In scope (files to create; nothing else changes):

• tests/test_extractors.py (create)
• tests/test_article_processor_flow.py (create)

Out of scope (do NOT touch):

• src/engine/extractors.py, src/engine/article_processor.py — these are
  characterization tests: they document current behavior. If current
  behavior looks wrong, STOP and report; do not "fix" the source to make a
  nicer test.
• tests/test_extraction_retry.py — leave the existing helper tests alone.

Git workflow

• Branch: advisor/006-extraction-characterization-tests
• Commit style: imperative subject, backticks around code identifiers.
• Do NOT push or open a PR unless the operator instructed it.

Steps

Step 1: tests/test_extractors.py

Patch src.engine.extractors.extract_entities_cloud,
src.engine.extractors.extract_entities_local, and (where prompt assembly is
asserted) src.engine.extractors.get_system_prompt. Tests:

1. Invalid type: EntityExtractor("animals") raises ValueError
   mentioning animals.
2. Cloud dispatch (default): extract("txt") with mocked cloud fn →
   cloud fn called once with entity_type="people" and model equal to
   src.constants.CLOUD_MODEL; local fn not called.
3. Local dispatch: extract("txt", model_type="ollama") → local fn
   called with model equal to src.constants.OLLAMA_MODEL; cloud not called.
4. Explicit model override: extract("txt", model="gemini/custom") →
   cloud fn receives model="gemini/custom".
5. Repair hint appended: patch get_system_prompt to return "BASE",
   call extract_cloud("txt", repair_hint="FIX IT") → cloud fn's
   system_prompt kwarg equals "BASE\n\nFIX IT".
6. No hint, no suffix: extract_cloud("txt") → system_prompt == "BASE".
7. Factories: each create_*_extractor() returns an EntityExtractor
   whose entity_type matches (4 quick asserts in one test).

Verify: uv run pytest tests/test_extractors.py -v → 7+ tests pass

Step 2: tests/test_article_processor_flow.py

First read extract_single_entity_type
(src/engine/article_processor.py:153-252) to confirm its exact
collaborators (which extractor method it calls, how QC results are consumed,
what the returned PhaseOutcome carries). Then write flow tests, patching at
the boundaries it actually uses (e.g. the extractor instance method and
run_extraction_qc in the src.engine.article_processor namespace if
imported at top level — confirm with grep -n "^from\|^import" src/engine/article_processor.py):

1. Happy path: extraction returns
   [{"name": "Alice Smith", "type": "detainee"}], QC returns no severe
   flags → PhaseOutcome success, extractor called exactly once.
2. Severe QC triggers exactly one retry with hint: first QC result
   carries ["zero_entities"], second is clean → extractor called twice,
   second call's repair_hint non-None (and contains the entity type per
   _build_repair_hint).
3. Retry does not loop: both QC results severe → extractor called
   exactly twice (not three times), outcome still returned (assert its
   success/failure flag matches current behavior — characterize, don't
   assume).
4. Extractor exception: extractor raises RuntimeError → characterize:
   does extract_single_entity_type catch it and return a failed
   PhaseOutcome, or propagate? Write the test to assert whichever the code
   actually does, with a comment naming the behavior.

Verify: uv run pytest tests/test_article_processor_flow.py -v → 4+ tests pass

Step 3: Full suite

Verify: just ci → exit 0

Test plan

This plan is a test plan — 11+ new tests across two files, modeled
structurally on tests/test_extraction_retry.py.

Done criteria

• just ci exits 0
• uv run pytest tests/test_extractors.py tests/test_article_processor_flow.py -v → 11+ pass
• grep -ln "EntityExtractor" tests/*.py → at least one match (was zero)
• No source files modified (git status shows only the two new test files + plans/README.md)
• plans/README.md status row updated

STOP conditions

Stop and report back (do not improvise) if:

• A characterization test reveals behavior that looks like a real bug (e.g.
  the repair hint is dropped on retry, or extract silently treats an
  unknown model_type as cloud — note: it currently DOES route any
  non-"ollama" value to cloud; that one is known and intentional-looking,
  characterize it). Report bugs; do not fix source here.
• extract_single_entity_type turns out to be untestable without invoking
  threading or real LLM helpers (i.e. its collaborators are not patchable at
  module level) — report what refactor would be needed instead of forcing it.

Maintenance notes

• Plan 010 (splitting mergers.py) and any extraction refactor should run
  these tests as their safety net; extend them when dispatch logic changes.
• Reviewer should scrutinize: tests assert which collaborator was called
  with what arguments (behavior), not internal attribute states.
• Deferred: tests for extract_all_entities's intra-article parallelism and
  check_relevance — worth a follow-up plan once these foundations exist.