Skip to content

Plan 010: Split the 1,351-line mergers.py into focused modules (behavior-preserving) #23

Description

@strickvl

Plan 010: Split the 1,351-line mergers.py into focused modules (behavior-preserving)

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/mergers.py src/engine/__init__.py
mergers.py is the highest-churn file in the repo — expect drift. If the
method inventory below no longer matches
grep -n " def \|^def \|^class " src/engine/mergers.py, re-derive the
module boundaries before cutting anything, and report what changed.

Status

  • Priority: P3
  • Effort: L
  • Risk: HIGH (large mechanical refactor of the pipeline's core)
  • Depends on: plans/006-extraction-characterization-tests.md (must be DONE first — and all existing merger tests must be green)
  • Category: tech-debt
  • Planned at: commit 5b5c634, 2026-06-12

Why this matters

src/engine/mergers.py is 1,351 lines — 4.5× the median engine module — and
mixes six jobs: lexical candidate blocking, embedding similarity search,
evidence-text construction, canonical-name selection, merge orchestration,
and new-entity creation. Its merge_entities method alone spans ~550 lines
(707–1259). Every change to any one concern risks the others, and the file's
churn history (12 commits of the last 50) shows it absorbs most feature
work. Splitting it into modules with single jobs makes each independently
testable and reviewable. This is a pure move refactor: zero behavior
change, proven by the existing test suite passing unmodified.

Current state

Method inventory of src/engine/mergers.py at the planning commit (line:
name):

44:  class MergeStats                          (dataclass + total())
59:  class _EvidenceWorkItem
73:  class _PendingProfileEmbedding
80:  def _batch_embed_texts                    (module function)
110: class EntityMerger:
141:   __init__(entity_type)
153:   _extract_key
160:   _format_key_for_display
165:   _get_search_embedding            (staticmethod)
176:   _get_search_embedding_meta
191:   _embeddings_compatible
214:   _lexical_text
224:   _score_canonical_name
231:   _pick_canonical_key
276:   _collect_entity_variant_texts
319:   _lexical_block
417:   find_similar_entity
523:   _add_alternative_name
577:   _extract_context_windows
635:   _build_evidence_text
707:   merge_entities                   (~550 lines — the orchestrator)
1260:  _create_new_entity
1338: def create_people_merger / _organizations_ / _locations_ / _events_
  • Public API that must NOT change: EntityMerger(entity_type) construction,
    merge_entities(...) full signature, MergeStats, the four factory
    functions, and whatever src/engine/__init__.py exports (check it).
  • Callers: src/process_and_extract.py (merge_and_finalize, line 606)
    constructs EntityMerger(entity_type) and calls merge_entities(...).
    Tests construct mergers directly: see tests/test_entity_merger_similarity.py
    and tests/test_entity_merger_merge_smoke.pyread both first; if
    they call private methods (_lexical_block, find_similar_entity, ...),
    those names must keep working on EntityMerger (thin delegation is fine).
  • Existing related modules to respect (don't duplicate): canonical-name
    scoring already lives in src/utils/name_variants.py
    (score_canonical_name) — _score_canonical_name wraps it; keep the
    wrapper with the selection logic.
  • Conventions: typing.Dict/typing.Tuple; engine modules exposed via
    src/engine/__init__.py; structured logging via existing log/
    log_decision helpers (move imports along with code).

Commands you will need

Purpose Command Expected on success
Merger tests uv run pytest tests/test_entity_merger_similarity.py tests/test_entity_merger_merge_smoke.py tests/test_merge_dispute_agent_routing.py tests/test_canonical_name.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
Size check wc -l src/engine/mergers.py < 700 at the end

Scope

In scope:

  • src/engine/mergers.py
  • src/engine/merge_similarity.py (create)
  • src/engine/merge_evidence.py (create)
  • src/engine/merge_canonical.py (create)
  • src/engine/__init__.py (re-export additions only)

Out of scope (do NOT touch):

  • ANY behavior change — no threshold tweaks, no "while I'm here" fixes, no
    renaming of config keys. If you spot a bug, write it in the report.
  • merge_entities's internal logic beyond replacing moved-method calls with
    delegated calls.
  • src/engine/merge_dispute_agent.py, match_checker.py, profiles.py.
  • Test files — they must pass unmodified (that is the proof of
    behavior preservation). Exception: import-path updates are forbidden too —
    keep old import paths working via EntityMerger delegation and
    __init__.py re-exports instead.

Git workflow

  • Branch: advisor/010-split-mergers-module
  • One commit per extraction step (3 commits + final cleanup) — each commit
    leaves the suite green.
  • Do NOT push or open a PR unless the operator instructed it.

Steps

Step 0: Baseline

Confirm plan 006 is DONE in plans/README.md and run the full suite.

Verify: just test → all pass (record the count; it must be identical at the end)

Step 1: Extract evidence building → src/engine/merge_evidence.py

Move _extract_context_windows (577–634) and _build_evidence_text
(635–706) plus _EvidenceWorkItem (59) into the new module as module-level
functions (convert self usage: they should need only entity display
text/keys and config values — pass those explicitly; read the bodies to
enumerate the exact inputs). In EntityMerger, keep same-named thin methods
that delegate, so internal call sites and any tests keep working:

def _build_evidence_text(self, ...):
    return merge_evidence.build_evidence_text(...)

Verify: just test → all pass, same count as Step 0

Step 2: Extract similarity search → src/engine/merge_similarity.py

Move _lexical_text, _collect_entity_variant_texts, _lexical_block,
find_similar_entity, _get_search_embedding, _get_search_embedding_meta,
_embeddings_compatible (lines 165–522) into a SimilarityMatcher class
constructed with entity_type (+ whatever per-call config those methods
take today — keep parameters, don't capture config at construction unless
EntityMerger.__init__ already does). EntityMerger instantiates one in
__init__ and delegates, preserving the existing method names on
EntityMerger (tests may call merger.find_similar_entity(...) /
merger._lexical_block(...) directly).

Verify: just test → all pass, same count

Step 3: Extract canonical-name selection → src/engine/merge_canonical.py

Move _score_canonical_name, _pick_canonical_key,
_add_alternative_name (lines 224–276 and 523–576). Same delegation
pattern. Keep using src/utils/name_variants.py functions — do not copy
them.

Verify: just test → all pass, same count

Step 4: Cleanup and re-exports

  • Remove now-unused imports from mergers.py (just format clears the
    easy ones; just lint flags the rest).
  • Add the three new modules to src/engine/__init__.py exports alongside
    the existing ones (match its current style — read it first).
  • Confirm mergers.py retains: MergeStats, _PendingProfileEmbedding,
    _batch_embed_texts, EntityMerger (with merge_entities,
    _create_new_entity, _extract_key, _format_key_for_display, and the
    delegation shims), and the four factories.

Verify: wc -l src/engine/mergers.py → < 700;
uv run python -c "from src.engine import EntityMerger; print('ok')" → ok;
just ci → exit 0

Test plan

No new tests and no modified tests — an UNCHANGED green suite is the
acceptance proof. Run the targeted merger tests after every move, full suite
at every step gate. If any test needed editing to pass, the refactor changed
behavior: revert that step.

Done criteria

  • just ci exits 0; test count identical to Step 0 baseline
  • git diff --stat shows zero changes under tests/
  • wc -l src/engine/mergers.py < 700
  • Three new modules exist; src/engine/__init__.py exports them
  • merge_entities signature byte-identical (git diff the signature region)
  • No files outside the in-scope list modified (git status)
  • plans/README.md status row updated

STOP conditions

Stop and report back (do not improvise) if:

  • Plan 006 is not DONE, or the baseline suite is not fully green.
  • The method inventory has drifted from the table above (new methods,
    moved boundaries) — re-derive and report the new split proposal before
    cutting.
  • A method slated for extraction turns out to mutate EntityMerger state
    (not just read config) — e.g. it writes to a cache attribute on self.
    Report it; moving stateful code changes the design, not just the layout.
  • Any test requires modification to pass at any step.
  • merge_entities (707–1259) itself: this plan does NOT decompose its
    internal flow. If you find that delegation forces restructuring inside it
    beyond call-site renames, stop.

Maintenance notes

  • The ~550-line merge_entities body is deliberately left intact — its
    decomposition (into pipeline stages: block → score → match → dispute →
    apply) is the natural follow-up plan once this layout settles.
  • Reviewer should scrutinize: that diffs are moves (use
    git diff --color-moved=dimmed-zebra — moved blocks should dim), and the
    delegation shims' argument forwarding (a swapped argument here corrupts
    merges silently).
  • Future config additions for similarity/blocking belong in
    merge_similarity.py, not back in mergers.py.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions