feat(act): measure realized savings for defer-* actions in act report

[AVSRPA1KR]

Follow-up to #631 (part 2 of the deferral-coverage work): make the defer-* actions measurable in act report.

The gap

The defer-enable / defer-alwaysload / defer-threshold plan kinds from #631 apply, journal, and undo correctly, but act report couldn't measure them — it returned "not measurable: no baseline captured at apply time", because report.ts never captured a baseline for these kinds nor knew how to compute their realized delta. The measurement piece the design called for was left unwired.

The fix

Deferral has the same effect as mcp-remove — MCP tool-def schema leaves the upfront prefix — so this mirrors that path, with one inversion in the realized signal:

• needsConfigBaseline + captureBaseline now cover the defer-* kinds. Servers are the named set for defer-alwaysload, or the observed MCP surface for defer-enable / defer-threshold (which re-enable deferral across everything). Per-session tokens use observed tool counts, or the 5-tools × 400 fallback, exactly like mcp-remove.
• A new deferRow computes realized savings as per-session prefix tokens × the post-apply sessions where deferral actually became active — detected by the same deferred-tools-inventory signal the mcp-deferral-off detector uses. ENABLE_TOOL_SEARCH is read at process start, so sessions begun before the client restarted still run deferral-off and are excluded; if none benefited, the row reports "not yet in effect" with zero realized, rather than claiming a saving that hasn't taken hold.

Records applied before this change (no baseline) keep the existing "no baseline captured at apply time" note, so nothing regresses.

Tests

11 new tests cover measured, partial, not-yet-in-effect, no-sessions, empty-baseline, and missing-baseline paths, plus baseline capture for each kind. Verified end to end locally: applying a defer-enable captures the baseline, and act report reports realized savings scaled to the sessions that adopted deferral (and honestly reports "not yet in effect" when none have).

Diff is limited to src/act/report.ts (+80) and tests/act-report.test.ts (+140). tsc --noEmit clean; the full act suite passes.

Note on CI: tests/cli-durable-totals.test.ts currently fails on main itself (reproduced on a clean checkout of 146037b) — it's in the daily-cache path, unrelated to this change, which only touches src/act/report.ts.

[ozymandiashh]

Nice work overall. The implementation follows the house style closely, mirrors the existing mcpRow / archiveRow shapes, stays deterministic, avoids any, and the 11 new tests all pass with tsc --noEmit clean on my machine.

Two things in the measurement itself I think need addressing before this lands, since the whole point of the feature is a number people can trust.

1. defer-enable / defer-threshold baselines can double count against mcp-remove

src/act/report.ts:651-654

function deferServers(finding: WasteFinding, ctx: CaptureCtx): string[] {
  if (finding.apply?.kind === 'defer-alwaysload') return finding.apply.servers.map(s => s.server)
  return observedMcpServers(ctx.projects)
}

For defer-enable and defer-threshold the baseline spans the entire observed MCP surface rather than a named subset, and captureBaseline's DEFER_KINDS branch then freezes per-server schema tokens for all of it.

If someone applies defer-enable and later mcp-remove on a server that was in that surface, computeActReport walks each journal record independently with no cross-record awareness, so both rows can claim the same server's schema tokens as realized savings over the same post-removal sessions, inflating totalRealizedTokens and the optimize headline.

That collides with the guarantee the report prints about itself:

Each fix measures only its own metric; effects are never attributed across signals.

This is more reachable than the older overlap cases because defer-enable / defer-threshold are the first kinds whose baseline deliberately covers the whole surface instead of named servers. Either scoping the baseline, or dedup-ing server attribution across records in computeActReport, would close it. If you would rather ship and document it, a note in HONEST_FOOTER would at least keep the printed claim accurate.

2. status: 'reverted' is asserted for a state the code says it cannot distinguish

src/act/report.ts:273-281

if (deferredSessions === 0) {
  return {
    ...base,
    estimatedForWindow,
    status: 'reverted',
    confidence,
    note: `not yet in effect: ... (takes effect on the next session; the client may not have restarted, or the change was reverted)`,
  }
}

The note is honest that these are two different situations, but the machine-readable field commits to the alarming one. And 'reverted' is the same value used at :249 for a verified config reversion and at :296 for "reverted by user: an archived item was moved back into place", so a --json consumer or anyone reading realizedCell cannot tell a genuine revert from "config is fine, just not exercised yet".

RealizedStatus at :58 is 'measured' | 'reverted' | 'not-measurable', so there is no neutral option today. A fourth state along the lines of 'pending' would let the note and the status agree.

3. Scope, which is really a maintainer call

The Measurement section of #614 asked for cache-hit rate before and after as a first-class reported number. This PR reports prefix-token savings only, which may well be the right first step, but the reduction is not called out anywhere.

Related, and again not mine to decide: #614 was closed as complete on 2026-07-20, and this PR opened on 2026-07-30 without a comment on it or a new tracking issue, so there is no recorded maintainer confirmation of the approach. Given CONTRIBUTING asks for that before feature work starts, it would be worth @iamtoruk saying explicitly whether continuing the closed issue's scope this way is fine, and whether token-only measurement is an acceptable cut or wants a follow-up issue.

Smaller notes

• captureBaseline's MCP_KINDS (:663-674) and DEFER_KINDS (:676-687) branches are identical apart from how servers is sourced, so they could share a helper.
• Object.values(baseline.metrics).reduce((a, b) => a + b, 0) is repeated verbatim in mcpRow (:235), deferRow (:262) and archiveRow (:289).
• No test covers the overlapping mcp-remove plus defer-enable case, nor separates "no MCP activity at all" from "MCP active but deferral off". Even pinning current behaviour would help.
• The "(part 2 of #614)" label in the comments and tests reads oddly, since #631 already carries that label for different work.

Happy to look again once the double-count question and the status value are settled.

[iamtoruk]

Seconding @ozymandiashh's review — I reproduced both findings against this branch, and they hold. The implementation itself is clean (house style, deterministic, 39 act-report tests pass, tsc clean), so this is close, but the two measurement issues should land first since the whole feature is a number people are meant to trust.

Finding 1 (double-count) — confirmed, reachable. deferRow sums perSessionTokens over the whole observed MCP surface (deferServers → observedMcpServers, report.ts:651-654), while mcpRow sums over the removed server, and computeActReport walks each journal record independently with no cross-record awareness. So defer-enable/defer-threshold followed by an mcp-remove on any server in that surface has both rows claim the same server's schema tokens over the same post-removal sessions, inflating totalRealizedTokens and the optimize headline — which contradicts the report's own printed "each fix measures only its own metric; effects are never attributed across signals." Dedup-ing server attribution across records in computeActReport is the real fix; a HONEST_FOOTER note is the minimum to keep the printed claim true.

Finding 2 (status honesty) — confirmed. report.ts:273-281 returns status: 'reverted' when deferredSessions === 0, but the note itself says the cause is ambiguous ("the client may not have restarted, or the change was reverted"), and 'reverted' is the same value used for genuine reversions (mcpRow:249, archiveRow:296). A --json consumer can't distinguish "config is fine, just not exercised yet" from a real revert. RealizedStatus (report.ts:58) has no neutral option; adding 'pending' lets the field agree with the note.

Scope (Finding 3): this reports prefix-token savings, not the cache-hit-rate #614 asked for as a first-class number, and #614 was closed before this opened. Whether prefix-tokens-as-first-step is the accepted scope is a maintainer product call rather than a code issue — flagging it needs an explicit decision before this lands, and the reduction should be called out wherever it ships.

Happy to re-review promptly once 1 and 2 are addressed.

[ozymandiashh]

Since both findings were confirmed with an endorsed fix shape and the branch allows maintainer edits, I pushed db018f7 implementing exactly that, so this doesn't stall on turnaround. @AVSRPA1KR feel free to amend or push back on any of it, it stays your PR.

What the commit does:

Finding 1 (double count): computeActReport now collects the servers named in the baselines of applied mcp-remove / mcp-project-scope records, and deferRow sums per-session tokens only over baseline servers outside that set. Scope rows are included alongside removals because a project-scoped server overlaps a whole-surface defer baseline the same way. Direction is deliberately conservative: a claimed server is dropped from the defer row for its whole window (even pre-removal sessions), so the report under-claims rather than over-claims and the footer's "each fix measures only its own metric" is literally true. When every server in a defer baseline is claimed, the row says so: not measurable: every server in this baseline is already measured by an MCP remove/scope row. The footer gained one sentence documenting the exclusion.

Finding 2 (status honesty): RealizedStatus gained 'pending', and the deferredSessions === 0 branch returns it instead of 'reverted' (note text unchanged). The table renders it as not yet in effect; --json consumers can now distinguish "config fine, not exercised yet" from a genuine revert. 'reverted' remains exclusively for the real reversion paths (mcpRow, archiveRow). Grepped the rest of src/ for RealizedStatus consumers: none outside act/report.ts, and yield's unrelated 'reverted' category is untouched.

Tests: the "not yet in effect" expectation moved to 'pending', plus three new cases: the disjoint-sum dedup (mcp row 10.0K + defer row 10.0K, total 20.0K where pre-fix it would have been 30.0K), the all-servers-claimed note, and the JSON pending/realizedTokens: null contract. 42/42 pass locally, tsc --noEmit clean, no new dependencies.

@iamtoruk this should be ready for a re-look. On your scope note (finding 3): agreed it's a product call; nothing in this commit widens or narrows what the PR measures.

Both findings addressed by db018f7 (dedup via mcpClaimedServers + pending status), verified locally and endorsed in shape by iamtoruk.

[ozymandiashh]

Approving as follow-through on the review cycle: both confirmed findings are fixed in db018f7 exactly along the shape @iamtoruk endorsed (cross-record server dedup in computeActReport; RealizedStatus pending for not-yet-in-effect). Verified: 42/42 act-report tests, tsc clean, CI 4/4 green, no new dependencies, no trailer issues. The fix commit is mine, so treat this approval as covering the original feature code by AVSRPA1KR, which I reviewed line by line in the first pass.