planner/optimizer: disambiguate retained-window count for QueryMethod::Subtract

[milindsrivastava1997]

planner/optimizer — `translator.rs::retention_count_for_assignment` (around line 60) hardcodes `1` for `QueryMethod::Subtract`, while `candidate_gen.rs` sets the deployed config's actual retained-window count to `n_windows`. Self-documented as a Phase-3 placeholder, but it's a silent mismatch waiting to bite once wired up.

Separately: for `Subtract`, retaining 1 aggregate makes sense for the current window, but we also need to retain another aggregate representing the sketch used for the previous query instance (to subtract against). It's possible this single calculation/parameter is being used to mean two different things and needs to be split into two distinct parameters.

Needs a design discussion to disambiguate before fixing.

Found during code review of PR #407.

---

Resolution

Traced every consumer of both fields before changing anything:

• AggregationConfig.num_aggregates_to_retain (set by candidate_gen.rs to n_windows for Subtract) is the engine-side retention cap — consumed for real by simple_map_store::per_key.rs/global.rs as the eviction cap on the checkpoint ring buffer. This was already correct: the engine needs n_windows depth so the checkpoint from range_a ago is still around when "now" arrives. No bug here.
• AggregationReference.num_aggregates_to_retain (hardcoded 1 in translator.rs) is the query-time combine count — how many retained checkpoints a query actually reads to compute its answer. For Subtract this is a fixed constant, 2 (current cumulative checkpoint + the one from range_a ago), independent of n_windows/range_a/W — that's what makes Subtract O(1). The doc comment on QueryMethod::Subtract already said this; the 1 was just a stale placeholder.
• These are two different concepts that happen to share a field name across two different structs (AggregationConfig vs AggregationReference), not one parameter doing double duty. No type/field split needed.
• Confirmed nothing currently executes Subtract's actual merge math at query time (window_merger.rs's IncrementalMerger for subtractable accumulators is still a documented future). So no consumer dictates a richer shape (e.g. an offset/stride field) yet — adding one now would be designing for a consumer that doesn't exist. Deferred until real Subtract execution lands and tells us what it needs.

Fix applied: translator.rs::retention_count_for_assignment now returns 2 for QueryMethod::Subtract instead of the placeholder 1.

Considered also renaming AggregationReference.num_aggregates_to_retain to something like num_aggregates_to_combine for clarity, since it means something different from AggregationConfig's field of the same name. Deferred for now — it has a real consumer (streaming_config.rs::from_yaml_data, used by the legacy generator.rs pipeline via planner_output.rs) keyed on that exact field/wire name, so the rename would need to either preserve the serde wire key or update that reader too. Out of scope for this fix.

[milindsrivastava1997]

Note for whoever wires up real Subtract execution: AggregationReference.num_aggregates_to_retain = 2 is a bare count, not "which 2." It doesn't say which checkpoints to combine — for Subtract that's specifically checkpoint_now and checkpoint_(now - n_windows), not just "the latest 2 contiguous ones" (that's the Merge interpretation, and it'd be wrong here). Deferred adding an explicit offset/stride field for this since nothing executes Subtract's merge yet (window_merger.rs's IncrementalMerger is still future work) — add it when that lands and tells us the real shape it needs.