fix(palace): file_already_mined iterates all groups when source_file has multiple parent_drawer_id mining passes

[milla-jovovich]

Summary

Under the additive-mining model (drawer history preserved across re-mines), a single source_file can have multiple parent_drawer_id groups in the palace — one per mining pass — each with its own stored source_mtime and normalize_version.

file_already_mined (the function the project miner uses to decide whether a file needs to be re-mined) previously used collection.get(where={"source_file": X}, limit=1) and only checked the single returned row. ChromaDB does not guarantee ordering for limit=1 across multiple matching rows, so the returned row was effectively arbitrary. When ChromaDB returned a stale group (older mining pass), the function returned False, the miner concluded the file had changed, and wrote yet another duplicate group of drawers for a file that had not actually changed.

This PR replaces the limit=1 shortcut with the paginated-iteration pattern the extract_mode is not None branch has always used. The function now returns True if ANY stored group is current (matching version + matching mtime), regardless of which group ChromaDB orders first.

Closes #1653.

What changes

• mempalace/palace.py — file_already_mined iterates all groups for the source_file in 1000-row pages, short-circuits on the first matching group. The two branches (extract_mode is None vs set) collapse into one loop that skips the extract_mode check when no mode is specified.
• tests/test_miner.py — adds test_file_already_mined_handles_multiple_groups_under_one_source_file, a deterministic regression test that uses a MockCollection to force worst-case ChromaDB ordering (returns stale group on limit=1) and asserts the function correctly returns True by iterating.

How the fix works

# Before — limit=1 shortcut, single row checked, ordering-dependent
results = collection.get(where={"source_file": X}, limit=1)
stored_meta = results.get("metadatas", [{}])[0] or {}
# ... check stored_meta version + mtime ...

# After — paginated iteration, short-circuit on first matching group
offset = 0
while True:
    results = collection.get(where={"source_file": X}, limit=1000, offset=offset, include=["metadatas"])
    for meta in results.get("metadatas") or []:
        # version check
        # mtime check (within 0.001s tolerance)
        if matches:
            return True
    if not (results.get("ids") or []):
        break
    offset += 1000
return False

Trade-off: average-case cost rises from O(1) limit=1 query to O(N/1000) paginated scan. For typical sources (1–3 groups) the cost is unchanged because the loop short-circuits on the first matching group within the first page. For pathological sources with thousands of groups, the cost is O(pages-until-match) — still bounded, no longer flaky.

Failure shape this prevents

t=0   file_mtime = 100
      mine() — writes group_A with stored source_mtime=100

t=1   user edits file, file_mtime becomes 200
      mine() — writes group_B with stored source_mtime=200

t=2   file unchanged since t=1
      mine() called → file_already_mined runs get(..., limit=1)
         - returns group_B → mtime 200 matches → True → skip → CORRECT
         - returns group_A → mtime 100 ≠ 200 → False → re-mine → WRONG
                              creates group_C, third duplicate

t=3   each spurious re-mine adds another group whose stale mtime can
      trigger the next spurious re-mine. Steady state: duplicate groups
      accumulate without bound for any file that has ever been edited.

Behavioral, not data-loss. Storage grows without bound; search results may show same content N times; closet pointers, hallway counts, and entity-frequency stats become inflated proportionally. Invisible until mempalace status reveals the bloat.

Test plan

RED-then-GREEN pinned deterministically. The new test uses a MockCollection that simulates two parent_drawer_id groups under one source_file:

• stale group (older source_mtime) returned for limit=1 calls (worst-case ordering)
• both groups returned for limit=1000 paginated calls

Against pre-fix code: test FAILS (function returns False because limit=1 picks stale group). Against post-fix code: test PASSES (iteration finds the current group).

Env	Result
macOS Python 3.12 (local)	✅ 2268 passed, 0 failed
Linux Python 3.9.25 (OrbStack)	✅ 2260 passed, 0 failed
Linux Python 3.11.15 (OrbStack)	✅ 2261 passed, 0 failed
Linux Python 3.13.13 (OrbStack)	✅ 2261 passed, 0 failed
ruff check + ruff format --check	✅ clean

Existing 4 file_already_mined tests continue to pass:

• test_file_already_mined_check_mtime
• test_file_already_mined_scopes_convo_extract_mode
• test_file_already_mined_extract_mode_paginates_large_sources
• test_file_already_mined_returns_false_for_stale_normalize_version

Backwards compatibility

• Public function signature unchanged
• Behavior for files with a single group is identical to before (single-page scan returns immediately on first match)
• Behavior for legacy palaces (pre-additive-mining, drawers without parent_drawer_id) is also unchanged — the function iterates one drawer and returns the same result it would have under limit=1

Provenance

Surfaced during the per-query audit on PR #1628's amendment cycle (the search for every bare where={"source_file": ...} query in the repo). One of six sites identified; the other five are legitimately file-global in intent (closet purges, full-rebuild deletes, paginated mode-filtered scans). This site is the one whose failure mode mirrors the cross-group stitching pattern PR #1628 fixed at the searcher layer.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request updates the file_already_mined function in mempalace/palace.py to correctly handle multiple groups under a single source_file. Instead of relying on a limit=1 query which has undefined ordering in ChromaDB and can return stale groups, the function now uses a paginated approach to iterate through all groups and check if any of them match the current file's modification time and version. Additionally, a unit test has been added in tests/test_miner.py to verify this behavior using a mock collection. There are no review comments, so no feedback is provided.