Plan 003: Persist deferred merge disputes to the review queue and add a CLI viewer

[strickvl]

Plan 003: Persist deferred merge disputes to the review queue and add a CLI viewer

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/merge_dispute_agent.py src/process_and_extract.py justfile
If any in-scope 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: P1
• Effort: S
• Risk: LOW
• Depends on: none
• Category: bug
• Planned at: commit 5b5c634, 2026-06-12

Why this matters

When an entity match falls in the "gray band" (similarity near the threshold,
match confidence low), a second-stage LLM — the merge dispute agent — decides
MERGE / SKIP / DEFER. The agent already contains fully built, tested code to
append DEFER cases to a JSONL review queue (review_queue_path parameter),
but the only caller never passes that parameter, so every deferred case —
exactly the ones flagged as needing a human eye — is silently dropped. The
agent also returns DEFER on API errors, so those are lost too. After this
plan: DEFER records land in {output_dir}/merge_review_queue.jsonl by
default, and just review-disputes pretty-prints them so a researcher can
actually triage.

Current state

• src/engine/merge_dispute_agent.py — the dispute agent.

  • run_merge_dispute_agent(...) (line 83) accepts keyword-only args
    including review_queue_path: Optional[str] = None (line 98).
  • On decision.action == MergeDisputeAction.DEFER and review_queue_path
    (line 155) it builds a record via _build_review_record(...) and calls
    append_merge_dispute_review_queue(review_queue_path, record) (line 169).
  • On any LLM exception it returns a DEFER decision with
    reason=f"Dispute agent API error: ..." (lines 142–148), which also
    flows through the persistence branch.

• src/engine/mergers.py — the only caller.

  • EntityMerger.merge_entities(...) (line 707) signature today:

    def merge_entities(
        self,
        extracted_entities: List[Dict[str, Any]],
        entities: Dict[str, Dict],
        article_id: str,
        article_title: str,
        article_url: str,
        article_published_date: Any,
        article_content: str,
        extraction_timestamp: str,
        model_type: str = "gemini",
        similarity_threshold: Optional[float] = None,
        domain: str = "guantanamo",
        domain_config: Optional[DomainConfig] = None,
    ) -> MergeStats:

  • The dispute-agent call (line 927) passes everything except
    review_queue_path:

    dispute_decision = run_merge_dispute_agent(
        entity_type=self.entity_type,
        new_name=self._format_key_for_display(entity_key),
        existing_name=self._format_key_for_display(similar_key),
        ...
        model_type=model_type,
        domain=domain,
        article_id=article_id,
    )

• src/process_and_extract.py — merge_and_finalize() (line 606) constructs
  EntityMerger(entity_type) and calls merger.merge_entities(...) with
  domain=processor.domain, domain_config=domain_cfg (see lines ~300–316).
  domain_cfg.get_output_dir() returns the domain output directory.

• justfile — task runner; recipes follow the pattern
  name *args:\n uv run python scripts/<script>.py {{args}}.

• scripts/ — utility scripts (e.g. scripts/list_domains.py); Rich is a
  transitive dependency already used for console output across the repo
  (from rich.console import Console, from rich.table import Table are
  available).

• Existing tests for the routing logic: tests/test_merge_dispute_agent_routing.py
  — read it before writing tests and reuse its mocking approach for LLM calls.

Commands you will need

Purpose	Command	Expected on success
Tests	just test	all pass
Targeted	uv run pytest tests/test_merge_dispute_agent_routing.py -v	all pass
Lint+format	just format && just lint	exit 0
Full CI parity	just ci	exit 0
Viewer smoke	uv run python scripts/review_disputes.py --domain guantanamo	table or "queue empty" message

Scope

In scope (the only files you should modify/create):

• src/engine/mergers.py (signature + pass-through only)
• src/process_and_extract.py (compute and pass the default path in merge_and_finalize)
• scripts/review_disputes.py (create)
• justfile (one new recipe)
• tests/test_merge_dispute_agent_routing.py (extend)

Out of scope (do NOT touch):

• src/engine/merge_dispute_agent.py — the persistence code already works;
  do not modify it.
• Any frontend route — a /disputes web page is a separate, future plan.
• Accept/override workflow — the viewer is read-only triage for now.

Git workflow

• Branch: advisor/003-wire-dispute-review-queue
• Commit style: imperative subject, backticks around code identifiers.
• Do NOT push or open a PR unless the operator instructed it.

Steps

Step 1: Thread review_queue_path through merge_entities

In src/engine/mergers.py:

1. Add review_queue_path: Optional[str] = None, to the
   merge_entities signature (after domain_config).
2. Add review_queue_path=review_queue_path, to the
   run_merge_dispute_agent(...) call at line 927.

Verify: grep -n "review_queue_path" src/engine/mergers.py → 2+ matches
(signature + call site); just lint → exit 0

Step 2: Pass a default path from the pipeline

In src/process_and_extract.py, inside merge_and_finalize() where
domain_cfg is resolved and before the per-entity-type merge loop, add:

review_queue_path = os.path.join(
    domain_cfg.get_output_dir(), "merge_review_queue.jsonl"
)

and pass review_queue_path=review_queue_path in the
merger.merge_entities(...) call. (os is already imported in this module.)

Verify: grep -n "merge_review_queue.jsonl" src/process_and_extract.py → 1 match; just test → all pass

Step 3: CLI viewer script

Create scripts/review_disputes.py:

• Args: --domain (default "guantanamo"), --limit (default 50, newest
  last). Resolve the queue path the same way the pipeline does:
  DomainConfig(domain).get_output_dir() + merge_review_queue.jsonl.
• If the file is missing or empty: print
  No deferred merge disputes for domain '<domain>'. and exit 0.
• Otherwise parse JSONL (skip and count malformed lines) and render a Rich
  Table with columns drawn from the record fields. Read
  _build_review_record in src/engine/merge_dispute_agent.py (it starts at
  the line after the run_merge_dispute_agent body, ~line 175) to get the
  exact field names — expect entity_type, new/existing names, similarity
  score + threshold, match confidence, agent reason, article_id, domain.
  Print a one-line summary at the end:
  <N> deferred disputes (<M> malformed lines skipped).
• No LLM calls, read-only — the script must work offline.

Add to justfile under the Data Management section:

# Show deferred merge disputes awaiting human review
review-disputes domain="guantanamo" *args:
    uv run python scripts/review_disputes.py --domain {{domain}} {{args}}

Verify: uv run python scripts/review_disputes.py --domain guantanamo →
exits 0 with either the table or the empty-queue message;
just review-disputes → same

Step 4: Tests

Extend tests/test_merge_dispute_agent_routing.py (reuse its existing LLM
mocking pattern — note from repo conventions: when a function is lazily
imported inside a function body, patch it at the source module; when
imported at module top level, patch it in the importing module's namespace.
Copy whatever the existing tests in this file already do — they pass in CI).

1. DEFER persists: mock the generation call to return a
   MergeDisputeDecision(action=DEFER, ...); call run_merge_dispute_agent
   with review_queue_path=str(tmp_path / "q.jsonl") → the file exists and
   contains exactly 1 JSON line whose entity_type / names match the input.
2. MERGE does not persist: same setup, decision MERGE → file does not exist.
3. No path, no write: DEFER with review_queue_path=None → no file
   created anywhere under tmp_path (and no exception).

Verify: uv run pytest tests/test_merge_dispute_agent_routing.py -v → all pass, including 3 new tests

Step 5: Full suite

Verify: just ci → exit 0

Test plan

Covered in Step 4 — three new tests in
tests/test_merge_dispute_agent_routing.py, modeled on that file's existing
mock style.

Done criteria

• just ci exits 0
• grep -n "review_queue_path=review_queue_path" src/engine/mergers.py → 1 match
• grep -n "merge_review_queue.jsonl" src/process_and_extract.py → 1 match
• scripts/review_disputes.py exists; just review-disputes exits 0
• 3 new tests pass in tests/test_merge_dispute_agent_routing.py
• 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:

• run_merge_dispute_agent no longer has the review_queue_path parameter,
  or the call at mergers.py:927 already passes it (drift — the finding was
  fixed independently).
• merge_and_finalize no longer constructs EntityMerger directly (the
  call chain changed).
• append_merge_dispute_review_queue turns out to do anything other than
  append a JSON line to a file (e.g. it needs a lock or directory that
  doesn't exist) — check whether it creates parent directories; if it
  doesn't and get_output_dir() may not exist yet, report rather than
  adding directory-creation logic to the agent module (which is out of scope).

Maintenance notes

• The queue only ever grows. A follow-up (deliberately deferred) is a
  resolve/dismiss workflow — either a --resolve <id> flag on the script or
  a frontend page — plus dedup of repeat disputes for the same entity pair
  across runs.
• If Plan 001 (checkpointing) and this land together, note the queue file is
  append-on-decision, not checkpoint-batched — a crash cannot lose queue
  entries that were already written.
• Reviewer should scrutinize: that the default-on behavior (always writing
  the queue file) is acceptable — it was chosen deliberately because the
  file only appears when DEFER actually happens.