feat(adblock): overlap report + source reorder (issue #215)

[flyhigher139]

Summary

Closes #215 — three commits that implement the three sub-tasks from the
issue, in the order I recommended in the issue analysis:

1. Regression tests pinning the priority contract (commit 1, +258 lines,
   zero production code) — promotes the "whitelist > nxdomain > zero_addr"
   chain in AdBlockEngine::check from a code comment to a CI-enforced
   contract. Also pins classify_rules's source-vec-order independence at
   the AdBlockState level (the actual property [Enhancement] 多源重叠规则的覆盖关系提示 + source 列表排序 #197's closure correction
   hinged on).

2. reorder_ad_block_sources IPC + ↑/↓ UI buttons (commit 2, +460 lines)
   — relative-move shape (not "rewrite the list"), boundary no-ops at the
   server, disabled buttons at the UI. Backend regression test asserts the
   resulting classify_rules output is identical pre/post swap.

3. get_ad_block_overlaps IPC + chip + drawer (commit 3, +882 lines) —
   per-source overlap count rendered as a pill on each source card; click
   opens a drawer listing the per-domain breakdown with the
   priority-derived effective badge. Lazy on the server (not bundled into
   get_ad_block_state) so the page-load IPC stays cheap.

What changed

Behaviour

• Users can now see when two blocklists cover the same domain, and which
  one wins under the priority chain.
• Users can now reorder their sources via ↑/↓ buttons.
• Source reorder is purely a presentation concern — never changes
  interception (verified end-to-end in reorder_does_not_change_classify_rules_output).

Engineering

• No production code change in check() or classify_rules (both were
  already correct per [Enhancement] 多源重叠规则的覆盖关系提示 + source 列表排序 #197's closure). The priority contract now has a
  regression test that fails loudly if anyone flips the order.
• New IPCs are pub(crate) and registered in lib.rs::tauri::generate_handler!.
• Frontend follows the existing atom/wrapper/component pattern; no new
  dependencies introduced.
• Project conventions preserved: onPointerDown={onPointerDown(() => {})}
  on every write button, disabled={isLoading || ...}, .catch(() => {})
  on every IPC call, error path through adBlockErrorAtom for mutation
  atoms and console.warn for non-fatal fetch atoms.

Tests

• Backend: 4 engine tests + 7 commands tests (3 in commit 1, 4 in
  commit 2 for reorder, 5 in commit 3 for overlap). All 205 backend tests
  pass (cargo test --lib).
• Frontend: 5 new tests (3 reorder + 2 overlap). All 335 frontend
  tests pass (pnpm test).
• Lint: cargo fmt, cargo clippy -D warnings, tsc, vite build all
  clean.

How to verify

# Backend
cd src-tauri
cargo fmt --all -- --check
cargo clippy --lib --all-features -- -D warnings
cargo test --lib

# Frontend
cd ..
pnpm install --frozen-lockfile
pnpm test
pnpm build

Out of scope

• Drag-and-drop reordering: the project has no draggable primitive in the
  stack. ↑/↓ buttons are sufficient, more accessible, and easier to test.
  Drag can be added later by composing repeated IPC calls (relative-move
  shape is forward-compatible).
• Splitting get_ad_block_overlaps into summary-only vs drill-down IPCs:
  current implementation returns both. For the typical 5-source / 100k-domain
  case the cost is < 100 ms and only fires on drawer open. If profile sizes
  grow beyond that, splitting is a trivial follow-up.
• The "re-fetch on every mutation" cadence for the overlap report is
  coarse-grained — each mutation atom that changes the source list could
  trigger it. Currently the report is fetched on mount only; manual refresh
  via the drawer's ×/reopen would re-fetch. Acceptable for v1.

Tracking

• Issue: [Enhancement] Ad-block 多源重叠的 UI 可观测性 + source 排序（承接 #197，后端已确定性无歧义） #215
• Tracking issue: [Tracking] Ad-block DNS-mode 后续优化（issue #130 review 拆分） #191
• Closed predecessor with corrected premise: [Enhancement] 多源重叠规则的覆盖关系提示 + source 列表排序 #197

[flyhigher139]

Self-review sanity check

Verification commands run from a clean state on codex/issue-215-adblock-overlap-and-reorder (HEAD 1c9e84d):

• cargo fmt --all -- --check — clean
• cargo clippy --lib --all-features -- -D warnings — clean
• cargo test --lib — 205 passed, 0 failed (matches PR description)
• pnpm build (= tsc && vite build) — clean
• pnpm test — flaky: 335 passed most runs, but clicking Down invokes the IPC with sourceId + direction 'down' fails intermittently with Number of calls: 0. See finding 1.

The architecture is sound, the priority contract is real and tight, the reorder invariant is genuinely proven end-to-end, the frontend conventions (onPointerDown, disabled, .catch) are followed consistently, and the CSS additions stay in the existing convention. Most of what this PR adds is good. Two real issues to address before merge, plus one coverage gap.

---

Finding 1 — Front-end test is flaky (CI risk)

src/pages/__tests__/AdBlock.test.tsx:761-779 ("clicking Down invokes the IPC with sourceId + direction 'down'") intermittently fails with expected "vi.fn()" to be called with arguments: [ 'src-a', 'down' ] / Number of calls: 0. I reproduced it ~3/30 runs of pnpm test; the matching "clicking Up" test on the line above passes every time.

Root cause: the assertion runs immediately after await act(async () => { fireEvent.click(downBtn); }), but the inner async function returns as soon as reorderSource(...) is called — the microtask that resolves await reorderAdBlockSources(...) inside the atom hasn't necessarily flushed yet. The Up test gets lucky more often because of test execution order; the Down test sits at the end of the suite and races more reliably.

Suggested fix (in the test):

import { waitFor } from "@testing-library/react";
...
await waitFor(() => {
  expect(mockReorderAdBlockSources).toHaveBeenCalledWith("src-a", "down");
});

Apply the same change to the Up test for symmetry. The PR description's claim "All 335 frontend tests pass" is statistically false — it's a flake that will burn CI minutes.

Finding 2 — Misleading docstring on fetchAdBlockOverlapsAtom

src/stores/profiles/actions.ts:625-643 documents:

Called on page mount (alongside the other fetch atoms) and on every source mutation (add / remove / enable / disable / response / rules_limit_override / reorder) so the chips stay in sync.

But the implementation in actions.ts (lines 679-783) shows none of the mutation atoms call fetchAdBlockOverlapsAtom — only the page's useEffect on mount does. So after the user adds/removes/toggles/reorders a source, the chip count and drawer details are stale until the next page load. The PR body itself flags this as out-of-scope ("Out of scope — The 're-fetch on every mutation' cadence … Currently the report is fetched on mount only … Acceptable for v1"), but the docstring on the action atom claims otherwise.

Two acceptable resolutions; either is fine:

1. Match the docstring to the code: rewrite the docstring to say "Called on page mount only".
2. Match the code to the docstring: have addAdBlockSourceAtom, removeAdBlockSourceAtom, setAdBlockSourceEnabledAtom, setAdBlockSourceResponseAtom, setAdBlockSourceRulesLimitOverrideAtom, and reorderAdBlockSourceAtom all await fetchAdBlockOverlapsAtom() after their set(adBlockStateAtom, state).

Recommend option 1 — the v1 trade-off is explicit and the code is the simpler artifact; misleading docs that say one thing while the code does another are worse than either alone.

Finding 3 — Non-blocking: whitelist branch of effective is untested end-to-end

src-tauri/src/commands/adblock.rs::overlap_report_marks_whitelisted_domains_as_effective_whitelisted (around line 2254) has only one source covering trusted.example.com. With one source, by_domain will have entries with sources.len() < 2, which are filtered by the continue at line 968. So the effective: "Whitelisted" branch in the details map is never asserted — the test only checks per_source[0].overlapping_domain_count == 0, which would pass even if the whitelist branch were deleted.

This is the same scenario the issue calls out ("两个 blocklists … 一个被 whitelist 覆盖 → 'Whitelisted'"). To actually pin the whitelist priority path, the test needs two sources covering the same whitelisted domain, e.g.:

// Two sources, both block `trusted.example.com`; whitelist
// suffix-matches it. The overlap entry's effective MUST be
// "Whitelisted" regardless of which block tier the sources use.
mhost_storage::adblock::write_cache(temp.path(), &s_za.source_id,
    b"0.0.0.0 trusted.example.com\n").unwrap();
mhost_storage::adblock::write_cache(temp.path(), &s_nx.source_id,
    b"0.0.0.0 trusted.example.com\n").unwrap();
let state = AdBlockState { enabled: true, sources: vec![s_za, s_nx],
    whitelist: vec!["trusted.example.com".into()], ..Default::default() };
let report = compute_overlap_report(&state, temp.path());
assert_eq!(report.details.get(&s_za.source_id).unwrap()[0].effective,
    "Whitelisted");

Worth adding now while the whitelist path is fresh.

---

What looks good (no action needed)

• Priority contract tests (priority_is_strictly_ordered_when_all_three_sets_coexist, nxdomain_wins_when_same_domain_is_in_both_block_sets, whitelist_beats_both_block_sets_on_same_domain) — tight assert_eq! with descriptive messages; a future refactor that flips the order fails them loudly. The existing pairwise tests (nxdomain_consulted_before_zero_addr, whitelist_overrides_everything) covered suffix-match semantics; these new ones cover the exact-same-domain case which was previously untested.
• reorder_does_not_change_classify_rules_output (commands::adblock::tests around line 2741) — actually proves what it claims: reads state, computes classify_rules partitions, performs the swap, persists, reloads, recomputes, asserts byte-identical partition sets. The end-to-end persist+reload round trip is the property that matters.
• reorder_ad_block_source_impl concurrency — write lock is held only for the swap (brief), then released before persist_and_reload takes a read lock. Same pattern as the existing mutation IPCs in this file; no new race introduced.
• Frontend conventions — the new ↑/↓ buttons, the overlap chip, and the drawer × button all have onPointerDown={onPointerDown(() => {})}, the buttons have disabled={isLoading || …}, and every IPC call has .catch(() => {}). Matches the rest of the page.
• ReorderDirection enum — two-element enum, #[serde(rename_all = "lowercase")], no i32 delta. Relative-move shape is the right call; the comment in the source explains the reasoning.
• CSS additions — stay in the existing --color-*, fallback) convention; no duplicate .btn/.card definitions. Class naming is consistent with the file.
• overlap_report_excludes_disabled_sources correctly pins the disabled-source contract.
• overlap_report_keeps_state_sources_order correctly pins the per_source ordering for frontend diff stability.

[flyhigher139]

Self-review follow-up: all three findings addressed

The three findings from my own self-review have been fixed in commit d5164a5. Summary of what changed and verification:

Fix 1 — Front-end test flake

Was: clicking Up/Down invokes the IPC tests asserted the IPC call immediately after await act(...). The atom's microtask that fires reorderAdBlockSources wasn't reliably flushed before the assertion; Down test failed ~10% of pnpm test runs.

Fix: Both assertions now wrapped in waitFor(...). Applied symmetrically to the Up test (which didn't flake but the inconsistency would have bitten later).

Verified: 5 consecutive pnpm test runs all pass. No flake.

Fix 2 — fetchAdBlockOverlapsAtom docstring

Was: Docstring claimed "called on every source mutation" but no mutation atom actually re-fetches the overlap report. Code's actual behavior was the v1 trade-off documented in the PR body's "Out of scope" section.

Fix: Per the review's option-1 recommendation, rewrote the docstring to match the code. The v1 trade-off (chip stays at mount-time values until next page load) is now documented at the action atom itself, not just in the PR body — and the docstring points readers at the follow-up paths if they want live updates.

Verified: Build clean.

Fix 3 — Whitelist priority branch untested

Was: overlap_report_marks_whitelisted_domains_as_effective_whitelisted only had ONE source, so by_domain filtered the entry at sources.len() < 2 and the effective: "Whitelisted" branch was never actually asserted.

Fix: New test overlap_report_marks_whitelisted_domain_as_effective_when_two_sources_cover_it — two sources covering the same whitelisted domain; asserts both details entries carry effective: "Whitelisted". Directly exercises the whitelist > nxdomain > zero_addr priority chain end-to-end through compute_overlap_report.

Verified: Test passes. cargo test --lib now reports 206 passed (was 205 before this commit — +1 from the new whitelist test).

Final verification

Command	Result
cargo fmt --all -- --check	clean
cargo clippy --lib --all-features -- -D warnings	clean
cargo test --lib	206/206 passed ✓
pnpm build	clean
pnpm test × 5 runs	335/335 every run (no flake)

Branch is now 4 commits: the original 3 + this fix. Ready to merge.