Skip to content

measure_rereview_duplication.py: "measured" spend leg over-counts when a lens's dispatch covers many files #2183

Description

@bdfinst

Context

Discovered during #2165's step 1.4 real-data validation run (see the results comment on #2164: #2164 (comment)).

scripts/measure_rereview_duplication.py's report subcommand, when given --transcript, substitutes a flat per-lens average real dispatch cost for the byte-based estimate on any lens the transcript actually dispatched (spend_source: "measured"). That average is applied to every duplicate (lens, file) pair individually.

The bug

A single review-agent dispatch typically reviews a whole changeset (many files) in one sitting — its real cost already amortizes across every file it touched. Applying that dispatch's flat average cost to each duplicate file separately multiplies one dispatch's real cost by however many files happen to be flagged as duplicates under that lens, rather than dividing it across them.

Confirmed against real data: at the whole-PR backstop checkpoint from #2180's build (216 duplicate pairs), the "measured" leg reported avoidable_tokens_estimate: 184,573,700 and total_tokens_estimate: 626,769,653 — but the transcript's own real total input spend across all 37 dispatches in the entire session was only 88,649,495 tokens. The measured leg's total for a single checkpoint's report is over 7x the real total spend of the whole session that produced it — an internal inconsistency that confirms the over-count.

The parallel byte-estimate leg (spend_source: "estimated", prices each file independently from its own size) doesn't have this defect and produced a materially lower, more trustworthy avoidable_pct_of_total for the same checkpoint (20.69% vs. the measured leg's 29.45%) — both still clear the AC's 5% threshold, so the qualitative conclusion in the #2164 comment is unaffected, but the "measured" number should not be trusted at face value for anything more precise than "directionally consistent."

Suggested fix

Redesign the per-lens cost attribution so it doesn't multiply-count: either (a) normalize by the average number of files a lens's dispatches actually covered per dispatch in the transcript (so the per-file share reflects a fair division of one dispatch's cost), or (b) report avoidable dispatch count as the primary "measured" metric rather than trying to attribute a per-file dollar figure at all, since dispatches — not files — are the actual unit of cost in /code-review//build.

Scope

scripts/measure_rereview_duplication.py's build_report/measured_avg_by_lens (added in #2165, PR TBD). Not a blocker for #2165 itself — the byte-estimate leg's numbers already support #2165's conclusion — but should be fixed before the "measured" leg's absolute figures are used for anything more precise than a rough cross-check, e.g. if epic #2164 slices 1-2 build a real ledger and want to validate against this tool's numbers.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-conventional-titleIssue title does not follow Conventional Commits

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions