[high] fix(mvt): index every detection MVT counted, not just the first 100

[elhoim]

BLUF

• Priority: high.
• run_mvt_android / run_mvt_ios report a detection count the case DB cannot honour. A device reported as having 250 spyware detections holds 100 that an examiner can actually find with search().
• Root cause: _collect_mvt_results sets module_counts[module] = len(data) — the number surfaced as detections and total_indicators — but appends only data[:100] to the indexed text, and only records that happen to be dicts.
• Nothing in the response says records were dropped. The count is the only signal the analyst has, and it overstates what is searchable.
• The ceiling is an oversight, not a resource control: the timeline CSVs collected in the same function are indexed whole and uncapped. The only capped data was the detection records themselves — the part that matters most.
• Fix: index every counted record, so module_counts and the indexed content agree by construction.
• Scope: src/mulder/server/tools/mvt.py, one function, +5/-3. No shared helper, no new abstraction.

The bug

    for result_file in sorted(Path(output_dir).rglob("*.json")):
        ...
            if isinstance(data, list):
                module_counts[result_file.stem] = len(data)   # <- counts all 250
                for item in data[:100]:                       # <- indexes 100
                    if isinstance(item, dict):                # <- and only dicts
                        parts.append(json.dumps(item, default=str))

raw_output then goes straight to extract_and_index with no further cap, so data[:100] is the only place evidence is lost — and it is upstream of the index, not a display limit.

Two distinct ways a counted record fails to arrive:

1. Position — record 101 onward is dropped. On a device with 250 hits in one module, 150 detections are counted and unfindable.
2. Type — a module emitting bare strings counts every element but indexes none of them.

Immediately below, in the same function, the timeline CSVs are indexed with no cap at all:

            text = timeline_file.read_text(encoding="utf-8", errors="replace")
            parts.append(f"=== {timeline_file.name} ===\n{text}")

That asymmetry is what identifies [:100] as an accident.

The fix

            if isinstance(data, list):
                module_counts[result_file.stem] = len(data)
                # Every counted record must reach the index: module_counts is
                # reported to the analyst as the number of findings, so any
                # record dropped here becomes a detection that is claimed but
                # cannot be found by search().
                parts.extend(json.dumps(item, default=str) for item in data)

The invariant is now testable directly, and one test asserts exactly it: sum(module_counts.values()) == len(raw_output.splitlines()).

Deliberately out of scope

• The format_records refactor from closed PR [high] fix(index): index the detections, not just how many there were #74. That branch added a shared formatter in extract_helpers.py and rewired nine tool modules through it. That is the cross-cutting shape the maintainer declined in [critical][feature] 0.2 — Introduce typed tool outcomes and coverage metadata #32, so this PR keeps the change inside mvt.py and touches no shared helper.
• The failure path. Open PR [high] fix(mvt): report a failed MVT run instead of a clean spyware scan #92 (fix/mvt-exit-code) handles a failed MVT run — it errors when MVT exits non-zero having produced nothing, and stops MVT's stderr being indexed as device evidence. This PR is the success path: MVT ran, produced findings, and the findings were partly discarded. The two are complementary and touch different code — [high] fix(mvt): report a failed MVT run instead of a clean spyware scan #92 adds a guard at the call sites, this changes the body of _collect_mvt_results. Verified conflict-free with git merge-tree --write-tree.
• run_photorec in extract/carving.py, the other module named in [high] fix(index): index the detections, not just how many there were #74. Checked and not defective for this concern: it indexes the full report.xml when present, and otherwise indexes real file paths, capping at _FILE_LIST_CAP = 500 with the truncation disclosed in the indexed text (... and N more). No silent loss, so no PR.

Verification

• uvx pre-commit run --all-files (new test staged first) → ruff, ruff-format, mypy all pass.
• uv run --locked --extra dev pytest tests/ -q → 862 passed, 1 deselected (test_disk_pcap.py::...::test_size_filtering_skips_large_pcaps, which writes a real 200 MB file and fails identically on unmodified main in this environment — a disk quota, not a regression).
• Discriminating check — with mvt.py restored to origin/main and the new tests kept, 4 of 8 fail and the 4 narrowness tests pass:

FAILED test_every_counted_detection_is_indexed
  - AssertionError: module_counts claims 250 detections but only 100 reached the index
FAILED test_a_detection_past_the_old_cap_is_searchable
  - AssertionError: a detection past the 100-record cap never reached the case DB
FAILED test_a_counted_non_dict_record_is_also_indexed - assert 1 == 2
FAILED test_the_count_and_the_index_agree_across_several_modules - assert 151 == 131

The narrowness tests pin that the fix does not over-reach: a clean device still yields no output, timeline CSVs are still indexed whole, a malformed module file is still skipped, and records stay one-per-line so an embedded newline cannot split one across index windows.

Context

Recovered from closed PR #74 (fix/summary-only-indexing). That branch was stacked on the exit-code work, so its mvt.py diff contained only classify_tool_exit plumbing — already delivered separately as #92. This defect was found by reading today's main rather than by replaying the closed diff, and is narrower than #74's framing: MVT does not index a bare summary, it indexes real records and then silently truncates them.

No outcome framework and no classify_tool_exit is reintroduced, per the review on #32:

focused fixes for specific tools that currently swallow failures or lose partial-result information would be welcome.

Branched fresh from current main (2e5432c); the closed branch was not revised in place.

🤖 Generated with Claude Code