Skip to content

detect_conflicts reports the same document pair once per chunk pair, so one disagreement prints four times #3

Description

@royalpinto007

What we want

detect_conflicts should report a document pair once, not once per retrieved chunk pair.

Why it matters

app/conflicts.py, detect_conflicts (line 43), compares hits pairwise:

official = [h for h in hits if h.is_official]
for i, a in enumerate(official):
    for b in official[i + 1 :]:
        if a.doc_id == b.doc_id:
            continue
        out.append(Conflict(doc_a=a.doc_id, doc_b=b.doc_id, ...))

The guard at line 55 only skips a document conflicting with itself. It does not stop the same pair of documents being reported repeatedly. Retrieval returns chunks, not documents, and search in app/retrieval.py happily returns several chunks from one document, so two official documents contributing two chunks each produce four identical Conflict entries for one real disagreement.

The user-facing effect is in app/cli.py lines 88 to 89, which prints one line per conflict:

conflict hr-policy vs finance-policy: two official documents both answer this; they may disagree
conflict hr-policy vs finance-policy: two official documents both answer this; they may disagree
conflict hr-policy vs finance-policy: two official documents both answer this; they may disagree
conflict hr-policy vs finance-policy: two official documents both answer this; they may disagree

The module docstring in tests/test_conflicts.py says it plainly: "a checker that cries wolf gets switched off". This is the checker crying wolf.

The staleness side of the same file already gets this right. detect_stale (line 86) keeps a seen: set[str] so each document is reported once, and there is a test for it, test_each_stale_document_is_reported_once_not_once_per_chunk (tests/test_conflicts.py line 101). Conflicts have no equivalent.

How to fix it

  1. In app/conflicts.py, dedupe by unordered document pair inside detect_conflicts, for example with a seen: set[frozenset[str]] in the same spirit as the seen set in detect_stale.
  2. Apply it to both loops. The official-versus-informal loop at lines 69 to 82 already has a break at line 82 that limits it to one flag per official document, but it can still emit the same pair twice when one official document appears as several chunks.
  3. Add a test in tests/test_conflicts.py next to test_chunks_from_one_document_are_not_a_conflict_with_themselves (line 64). The existing _hit helper (line 23) makes this a two line test: build two chunks each for two official doc ids, and assert len(detect_conflicts(hits)) == 1.

Running it

Tests need a Postgres with pgvector for the async cases, but the conflict tests above are pure functions:

pytest tests/test_conflicts.py -q -k "conflict"

Comment here if you would like to take this one and I will assign it. I usually reply within a day.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinggood first issueGood for newcomers

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions