refactor(mcp): extract coverage and target-coverage sizing into pkg/recfilter (T4+T5)

[cristim]

Closes #1886

Stacked on #1885 (base branch is feat/mcp-recfilter-audit, not main). Review #1885 first; the diff shown here is only this slice.

Second slice of the MCP CLI-parity extraction (docs/plans/mcp/01-parity.md T4 + T5). Pure extraction: no behaviour change.

Why this one matters more than a normal refactor

Both moved functions are on the money-sizing path, and both are easy to reimplement plausibly but wrongly:

• ApplyCoverage scales an RI's cost-bearing fields by the discrete ratio newCount/rec.Count, not by the requested coverage/100. Count=3 at coverage=50 gives Count=1 — 1/3 of the instances. Scaling the money by 0.5 instead would overstate the sized purchase by ~50%. pkg/recfilter/sizing_test.go pins this as an explicit regression test.
• ApplyTargetCoverage does existing-aware per-pool floor arithmetic against ExistingCoveragePct, with clamped projection metrics and several no-signal passthroughs (avg<=0, RecommendedUtilization<=0, HourlyCommitment<=0, wrong-type or typed-nil Details).

Rather than let mcp/tools grow a second implementation, both move into pkg/recfilter/sizing.go.

What changed

• New pkg/recfilter/sizing.go: ApplyCoverage(recs, coverage, logf, drops) and ApplyTargetCoverage(recs, targetPct, logf, drops), plus the unexported applyTargetCoverageOne / RI / SP branches. Bodies and doc comments moved verbatim; every AppLogger.Printf became logf.printf with the identical format string.
• cmd/helpers.go: ApplyCoverage, applyCoverage and ApplyTargetCoverage become thin wrappers passing AppLogger.Printf. Signatures unchanged, so applySizing and the existing cmd sizing tests are untouched. The unexported branch functions are deleted (no cmd test called them directly — checked).
• The ApplyCoverage/applyCoverage pair collapses to one recfilter function taking drops. The split only existed to give the exported form a shorter signature.

stdout is the MCP transport

cmd/helpers.go declares var AppLogger = log.New(os.Stdout, ...). Nothing in pkg/recfilter may reference a package-level logger, so every logging call site takes the injected Logf introduced in #1885. nil is silent, and that is covered by tests on both functions.

Tests

pkg/recfilter/sizing_test.go, 28 cases:

• the discrete-ratio regression (Count=3, coverage=50 -> Count=1 with money at exactly 1/3)
• coverage >= 100 no-op, coverage <= 0 empty
• SP scales HourlyCommitment, not Count
• wrong-type and typed-nil Details pass through unscaled with exactly one warning (an interface holding a typed nil satisfies the type assertion, which is why the code checks ok && details != nil)
• sized-to-zero records DropTargetSizedToZero; drops == nil does not panic
• gap <= 0 -> DropTargetAlreadyMet; floor-to-zero -> DropTargetSizedToZero; no-signal passthrough
• Count == 0 with positive avg produces no NaN/Inf in any cost field
• SP edge passthroughs leave ProjectedUtilization at zero (setting projections on a rec whose money could not be scaled would produce a misleading row)
• unsupported CommitmentType warns exactly once per type
• targetPct outside (0,100] returns recs unchanged with one warning
• nil logf safe on every path

go build ./..., go test ./cmd/... ./pkg/..., go vet ./..., gocyclo -over 10 all clean.

One deviation from the plan

The plan lists "Projected* clamp 100" as a T5 test. ProjectedUtilization overflows easily and is tested. ProjectedCoverage cannot exceed 100 under this formula: nTarget = floor(avg * gap / 100) implies nTarget/avg*100 <= gap, so projCov <= existing + gap = targetPct <= 100. Its clamp only guards float epsilon. The test asserts the target=100 boundary lands exactly at 100 rather than fabricating an unreachable overflow case.

Follow-ups

DuplicateChecker (T6), then the MCP JSONL audit log (T2).

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 615e0fef-875c-4c8e-be67-46cfb6e74bf7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

For the record: GitHub marked this PR "merged" automatically when its base branch feat/mcp-recfilter-audit was fast-forwarded to include this commit. It has NOT landed on main.

The work is consolidated into #1885 (base main), which carries T1, T3, T4, T5 and T6 as five atomic commits. Reason for consolidating: this repo gates CI and CodeRabbit on the base branch, so stacked PRs get neither. #1886 is reopened and closed by #1885.