From 7adb27233086b324f9637591bc1655535f4f9951 Mon Sep 17 00:00:00 2001 From: zz_y Date: Wed, 26 Aug 2026 13:51:17 -0600 Subject: [PATCH 1/2] feat(dag-viewer): planner cost/benefit annotations (#286) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a structured, optional CostAnnotation schema (crates/types/src/cost.rs) and wires it through dag_export's JSON output and the tools/dag-viewer sidebar/on-graph UI, per issue #286. Rust: - `asap_types::cost`: `CostAnnotation` (value/unit/source/baseline/delta/ benefit_ratio/model_version/benchmark_id/inputs), `CostUnit` (CostUnitsPerSecond / CostUnits / RelativeStructuralUnits), `CostSource` (Modeled/Measured/Unavailable), `BaselineRef`, `CostInput`, `total_cost(rate, horizon, one_shot)`, `sum_workload_costs` (dedups by an explicit key, rejects unit-mismatched aggregation), and `WorkloadCostSummary`/`workload_cost_summary`. - `DagGraph` gains `edge_annotations: Vec`, populated by `deduplicate_pointer_shared_nodes` for every edge running into a genuine DAG merge point (never a guessed multi-hop path cost). - `DagDecision` and `TargetReplacement` gain `baseline_cost`/`selected_cost`/ `benefit` alongside their existing bare `cost: f64` (unchanged, for backward compat). `NamedGraph`/`WorkloadGraph` gain `workload_cost: Option`. - crates/devtools/src/bin/dag_export.rs populates all of the above from today's `asap_aware_mapping::cost_model` output (`estimate_cost`, `default_cse_recompute_cost`), deduplicating workload totals by `decision.id`. Every value dag_export produces today is honestly unit-tagged `RelativeStructuralUnits` (the same structural-size proxy the cost model already uses for ranking), not `CostUnitsPerSecond`: the cost model has no update_rate/evaluation_rate/query_interval recurrence inputs yet (#287's job). The annotation plumbing accepts a real rate unchanged once #287 lands those inputs. Nothing is ever fabricated: a value the cost model can't estimate is `CostSource::Unavailable` (`value: None`), never `0`. JS/viewer: - viewer.js renders concise on-graph `▼NN%`/`▲NN%` badges on a costed post-ASAP node's label, full baseline/selected/benefit/provenance blocks in the sidebar (node click, edge click, and a workload-scope cost summary for the current single- or multi-query selection), all sourced only from explicit JSON fields (decision.id dedup, `EdgeCostAnnotation`, `workload_cost`) — no client-side cost estimation. - index.html: cost UI CSS (light/dark aware, via existing --var tokens). - tools/dag-viewer/dag.example.json regenerated via generate-sample.sh (real lowering -> ASAP-aware mapping -> post-ASAP -> dag_export pipeline), not hand-patched. - README.md documents the new JSON contract fields. Tests: `cargo test --workspace` (all green) and `python3 -m unittest discover -s tools/dag-viewer -p test_render.py` (18/18) both pass. Co-Authored-By: Claude Sonnet 5 --- crates/devtools/src/bin/dag_export.rs | 169 +++- crates/types/src/cost.rs | 519 ++++++++++ crates/types/src/dag_export.rs | 205 +++- crates/types/src/lib.rs | 1 + tools/dag-viewer/README.md | 38 +- tools/dag-viewer/dag.example.json | 1270 ++++++++++++++++++++++++- tools/dag-viewer/index.html | 34 + tools/dag-viewer/render.py | 11 +- tools/dag-viewer/viewer.js | 176 +++- 9 files changed, 2387 insertions(+), 36 deletions(-) create mode 100644 crates/types/src/cost.rs diff --git a/crates/devtools/src/bin/dag_export.rs b/crates/devtools/src/bin/dag_export.rs index 3dd7dde..e5f9ab4 100644 --- a/crates/devtools/src/bin/dag_export.rs +++ b/crates/devtools/src/bin/dag_export.rs @@ -53,8 +53,9 @@ use std::collections::HashMap; use std::rc::Rc; use std::time::Instant; -use asap_aware_mapping::cost_model::DefaultCostModel; +use asap_aware_mapping::cost_model::{default_cse_recompute_cost, DefaultCostModel}; use asap_aware_mapping::replacement::{search_workload, Replacement, ReplacementSubDAG}; +use asap_types::cost::{BaselineRef, CostAnnotation, CostInput, CostSource, CostUnit}; use asap_types::dag_export::{ self, DagDecision, DagGraph, DagNote, NamedGraph, PostAsapSubstitution, TargetReplacement, TargetReplacementAfter, WorkloadGraph, @@ -65,6 +66,83 @@ use asap_types::pre_asap::query_expr::QueryExpr; use asap_types::pre_asap::schema::{Column, DataType, Schema}; use asap_types::types::AccuracyTarget; +/// `model_version` tag for every [`CostAnnotation`] this binary computes — +/// see [`winner_cost_annotations`]'s own doc for what it's a structural +/// proxy for and why (issue #286, issue #287 is the real rate-modeling +/// follow-up). +const STRUCTURAL_COST_MODEL_VERSION: &str = "dag_export-structural-cost-v1"; + +/// Baseline/selected/benefit [`CostAnnotation`]s for one [`Winner`] — issue +/// #286's "replacement-region baseline cost, selected cost, and benefit" +/// granularity item, reused verbatim for [`TargetReplacement`] and for the +/// [`DagDecision`] carried by every node the winning candidate produced or +/// carried. +/// +/// The baseline is always [`BaselineRef::PreAsapRecomputation`]: the cost of +/// recomputing `target` independently at every one of its `consumer_count` +/// use sites, via +/// [`default_cse_recompute_cost`] — the exact structural-size proxy +/// `asap_aware_mapping::cost_model`'s own `DefaultCostModel` already uses, +/// not a second formula. This baseline is always computable (never +/// `Unavailable`) since it only needs `target` itself, unlike `selected`, +/// which is `Unavailable` whenever `selected_cost` is `NaN` (the plugged-in +/// cost model has no numeric estimate for that particular candidate shape — +/// see [`RankedGroup::costs`](asap_aware_mapping::replacement::RankedGroup::costs)'s +/// own doc). +/// +/// Every value here is unit-tagged [`CostUnit::RelativeStructuralUnits`], +/// not [`CostUnit::CostUnitsPerSecond`]: today's cost model has no +/// `update_rate`/`evaluation_rate`/`query_interval` recurrence inputs at all +/// (issue #287's job) — see `asap_types::cost`'s module doc for why this +/// crate refuses to mislabel a structural proxy as a real rate. +fn winner_cost_annotations( + target: &QueryExpr, + consumer_count: usize, + selected_cost: f64, +) -> (CostAnnotation, CostAnnotation, CostAnnotation) { + let per_consumer_recompute = default_cse_recompute_cost(target).0; + let baseline_value = per_consumer_recompute * consumer_count.max(1) as f64; + let baseline = CostAnnotation::modeled( + baseline_value, + CostUnit::RelativeStructuralUnits, + STRUCTURAL_COST_MODEL_VERSION, + vec![ + CostInput::new("per_consumer_recompute_cost", per_consumer_recompute), + CostInput::new("consumer_count", consumer_count.max(1) as f64), + ], + ); + + if !selected_cost.is_finite() { + return ( + baseline, + CostAnnotation::unavailable(CostUnit::RelativeStructuralUnits), + CostAnnotation::unavailable(CostUnit::RelativeStructuralUnits), + ); + } + + let selected = CostAnnotation::modeled( + selected_cost, + CostUnit::RelativeStructuralUnits, + STRUCTURAL_COST_MODEL_VERSION, + vec![], + ) + .with_baseline(BaselineRef::PreAsapRecomputation, baseline_value); + + let benefit = CostAnnotation { + value: selected.delta, + unit: CostUnit::RelativeStructuralUnits, + source: CostSource::Modeled, + baseline: Some(BaselineRef::PreAsapRecomputation), + delta: None, + benefit_ratio: selected.benefit_ratio, + model_version: Some(STRUCTURAL_COST_MODEL_VERSION.to_string()), + benchmark_id: None, + inputs: Vec::new(), + }; + + (baseline, selected, benefit) +} + use asap_devtools::{lower_promql, lower_sql, SqlCatalog}; enum Lang { @@ -251,6 +329,10 @@ struct Winner<'a> { target: &'a Rc, candidate: &'a ReplacementSubDAG, cost: f64, + /// The target's own `MemoGroup::consumer_count` — threaded through so + /// [`winner_cost_annotations`] can compute a baseline without a second + /// lookup back into `PlanSpace`. + consumer_count: usize, } /// Short explanation intended for a selected winner in node-level UI. The @@ -316,6 +398,30 @@ fn lookup_winner( .find(|&i| expr == winners[i].target.as_ref()) } +/// One `(decision.id, baseline_cost, selected_cost)` triple per *distinct* +/// [`DagDecision`] carried anywhere in `graph` — collapsing every node that +/// shares one `decision.id` (a replacement region can span many nodes, all +/// carrying an identical clone of the same decision) down to a single +/// entry, so a caller summing these never counts one decision's cost once +/// per node it happens to touch. +fn decision_cost_entries(graph: &DagGraph) -> Vec<(u32, CostAnnotation, CostAnnotation)> { + let mut seen = std::collections::HashSet::new(); + let mut entries = Vec::new(); + for node in &graph.nodes { + let Some(decision) = &node.decision else { + continue; + }; + if !seen.insert(decision.id) { + continue; + } + let (Some(baseline), Some(selected)) = (&decision.baseline_cost, &decision.selected_cost) else { + continue; + }; + entries.push((decision.id, baseline.clone(), selected.clone())); + } + entries +} + /// Build a [`TargetReplacement`] for `winner`, matching this file's own /// per-target `before`/`after` construction. fn target_replacement( @@ -333,6 +439,8 @@ fn target_replacement( TargetReplacementAfter::Rewrite(dag_export::export(rewritten)) } }; + let (baseline_cost, selected_cost, benefit) = + winner_cost_annotations(winner.target, winner.consumer_count, winner.cost); TargetReplacement { decision_id, target_pre_id, @@ -342,6 +450,9 @@ fn target_replacement( cost: winner.cost, before, after, + baseline_cost: Some(baseline_cost), + selected_cost: Some(selected_cost), + benefit: Some(benefit), } } @@ -461,6 +572,7 @@ fn run_post_asap_with_progress( target: group.target, candidate, cost: group.costs[0], + consumer_count: group.consumer_count, }) }) .collect(); @@ -503,6 +615,8 @@ fn run_post_asap_with_progress( let mut find_winner = |expr: &QueryExpr| -> Option { let i = lookup_winner(&by_hash, &winners, &mut post_graph_cache, expr)?; let winner = &winners[i]; + let (baseline_cost, selected_cost, benefit) = + winner_cost_annotations(winner.target, winner.consumer_count, winner.cost); let decision = DagDecision { id: i as u32, strategy: winner.candidate.strategy.to_string(), @@ -510,6 +624,9 @@ fn run_post_asap_with_progress( rank: 0, cost: winner.cost, role: "replacement_region", + baseline_cost: Some(baseline_cost), + selected_cost: Some(selected_cost), + benefit: Some(benefit), }; Some(match &winners[i].candidate.replacement { Replacement::Rewrite(rc) => PostAsapSubstitution::Rewrite { @@ -675,6 +792,7 @@ async fn main() { graph, replacements: Vec::new(), post_graph: None, + workload_cost: None, }); } for (explanation, matched) in explanations.iter().zip(matched) { @@ -718,7 +836,54 @@ async fn main() { assign_workload_node_ids(&mut post_graphs); } - let workload = WorkloadGraph { queries }; + // Whole selected-workload cost/benefit (issue #286) — per query, and + // for the whole selected workload. `decision.id` (== the winning + // `Winner`'s own index — see `run_post_asap_with_progress`) is already + // a collision-free dedup key for a decision shared across multiple + // nodes (a replacement region spans several nodes, all carrying the + // same `decision.id`) and across multiple queries (a CSE-shared target + // reachable from more than one query's root) alike, so it's reused + // directly as `sum_workload_costs`'s dedup key — no separate lookup + // needed. + let mut workload_entries = Vec::new(); + for query in &mut queries { + let Some(post_graph) = &query.post_graph else { + continue; + }; + let entries = decision_cost_entries(post_graph); + if entries.is_empty() { + continue; + } + match asap_types::cost::workload_cost_summary( + entries.iter().map(|(id, baseline, selected)| (Some(*id), baseline, selected)), + "dag_export-workload-cost-v1", + ) { + Ok(summary) => query.workload_cost = Some(summary), + Err(mismatch) => eprintln!("dag_export: workload cost aggregation for {:?} skipped — {mismatch}", query.name), + } + workload_entries.extend(entries); + } + let workload_cost = if workload_entries.is_empty() { + None + } else { + match asap_types::cost::workload_cost_summary( + workload_entries + .iter() + .map(|(id, baseline, selected)| (Some(*id), baseline, selected)), + "dag_export-workload-cost-v1", + ) { + Ok(summary) => Some(summary), + Err(mismatch) => { + eprintln!("dag_export: workload-wide cost aggregation skipped — {mismatch}"); + None + } + } + }; + + let workload = WorkloadGraph { + queries, + workload_cost, + }; if progress { eprintln!( "Total planner time: {:.2} ms", diff --git a/crates/types/src/cost.rs b/crates/types/src/cost.rs new file mode 100644 index 0000000..0ddc84e --- /dev/null +++ b/crates/types/src/cost.rs @@ -0,0 +1,519 @@ +//! Structured cost/benefit annotations for [`dag_export`](crate::dag_export) +//! output — issue #286. +//! +//! The issue's cost/benefit semantics are defined in **cost units per +//! second** for recurring costs: +//! +//! ```text +//! maintained_cost_rate = +//! update_rate * maintenance_cost_per_update +//! + evaluation_rate * summary_read_cost +//! +//! recompute_cost_rate = evaluation_rate * raw_recompute_cost +//! evaluation_rate = sum(1 / query_interval_i) +//! +//! estimated_benefit_rate = baseline_cost_rate - selected_cost_rate +//! estimated_benefit_ratio = estimated_benefit_rate / baseline_cost_rate +//! ``` +//! +//! Today's cost model (`asap_aware_mapping::cost_model`) has no notion of +//! `update_rate`/`evaluation_rate`/`query_interval` at all — issue #287 +//! ("recurrence-aware modeled inputs") is what's expected to supply those. +//! Rather than inventing rate numbers this crate has no basis for, every +//! annotation this module produces from today's cost model is honestly +//! unit-tagged [`CostUnit::RelativeStructuralUnits`] — the *same* underlying +//! numbers `asap_aware_mapping::cost_model::CostModel::estimate_cost` and +//! `default_cse_recompute_cost`/`default_cse_shared_maintenance_cost` +//! already compute (a structural-size proxy, not a real cost-per-second +//! rate) — so a renderer can never mistake a structural proxy for a +//! measured or rate-modeled cost. The formulas above still hold shape for +//! shape (`benefit = baseline - selected`, `ratio = benefit / baseline` +//! guarded at `baseline <= 0`), just over whichever [`CostUnit`] the inputs +//! actually carry; when #287 lands real rates, a caller can construct a +//! [`CostAnnotation`] with `unit: CostUnit::CostUnitsPerSecond` through the +//! exact same type with no further plumbing change needed here. +//! +//! Nothing in this module ever fabricates a number: a hook with no basis to +//! estimate returns [`CostAnnotation::unavailable`] (`value: None, source: +//! CostSource::Unavailable`), never `0.0` or another synthetic placeholder. + +use serde::{Deserialize, Serialize}; + +/// The unit one [`CostAnnotation::value`] (and its `delta`) is expressed in. +/// See the module doc for why today's `dag_export` output uses +/// [`RelativeStructuralUnits`](CostUnit::RelativeStructuralUnits) rather than +/// claiming a real rate it can't back up. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum CostUnit { + /// A recurring cost expressed per second (`cost_units / s`) — the + /// `*_cost_rate` quantities in the issue's formulas. Only a + /// [`CostSource::Modeled`] annotation backed by real recurrence inputs + /// (issue #287) or a [`CostSource::Measured`] benchmark (issue #288) + /// may use this unit. + CostUnitsPerSecond, + /// A finite-run or one-shot total at some horizon `H` — `total_cost(H)` + /// in the issue's formulas, or a standalone one-shot addend. Never + /// aggregated with [`CostUnitsPerSecond`](CostUnit::CostUnitsPerSecond) + /// except through [`total_cost`], which keeps the two terms explicit + /// rather than silently adding a rate to a total. + CostUnits, + /// A dimensionless structural-size proxy (e.g. unique-DAG-node count, + /// the same magnitude + /// `asap_aware_mapping::cost_model::default_cse_recompute_cost` already + /// returns) — used wherever the underlying cost model has no rate or + /// absolute-cost estimate at all yet (issue #287). Never comparable to + /// [`CostUnitsPerSecond`](CostUnit::CostUnitsPerSecond) or + /// [`CostUnits`](CostUnit::CostUnits): a renderer must show it as its + /// own kind of number, and [`sum_workload_costs`] refuses to aggregate + /// mismatched units rather than silently mixing them. + RelativeStructuralUnits, +} + +impl std::fmt::Display for CostUnit { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + CostUnit::CostUnitsPerSecond => write!(f, "cost units/s"), + CostUnit::CostUnits => write!(f, "cost units"), + CostUnit::RelativeStructuralUnits => write!(f, "relative structural units"), + } + } +} + +/// Provenance of one [`CostAnnotation`]'s value — the issue's three states. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum CostSource { + /// Generated by a named/versioned cost model + /// ([`CostAnnotation::model_version`]). + Modeled, + /// Loaded from a reproducible benchmark artifact + /// ([`CostAnnotation::benchmark_id`]) — issue #288. + Measured, + /// No value. `value` is always `None` for this source — never encode an + /// unknown value as `0` or another synthetic number. + Unavailable, +} + +/// What a [`CostAnnotation`]'s `baseline`/`delta`/`benefit_ratio` are +/// measured against. The issue names two examples explicitly; both are +/// first-class variants here rather than opaque strings so a renderer can +/// display them without guessing. [`Named`](BaselineRef::Named) covers any +/// other explicitly-chosen baseline a future caller introduces. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(tag = "kind", content = "detail")] +pub enum BaselineRef { + /// Recomputing the pre-ASAP target independently at every consumer site + /// — "do nothing" (never apply ASAP-aware replacement at all). + PreAsapRecomputation, + /// The best-ranked *non-selected* legal candidate for the same target + /// (`rank` into that target's own `PlanSpace::cost_sorted` ordering, + /// `0` = best; a baseline referencing this variant is always `rank >= + /// 1`, since `rank 0` is what got selected). + HighestRankedNonSelectedCandidate { rank: usize }, + /// Any other explicitly-named baseline. + Named(String), +} + +/// One raw input that fed a [`CostAnnotation`]'s `value` — surfaced so the +/// viewer's sidebar can show *why* a modeled number is what it is, not just +/// the number itself. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct CostInput { + pub name: String, + pub value: f64, + #[serde(skip_serializing_if = "Option::is_none")] + pub unit: Option, +} + +impl CostInput { + pub fn new(name: impl Into, value: f64) -> Self { + CostInput { + name: name.into(), + value, + unit: None, + } + } +} + +/// A structured, optional cost/benefit annotation — issue #286's schema. +/// Every numeric value carries units and provenance; a missing value is +/// [`CostSource::Unavailable`] with `value: None`, never a fabricated +/// number. See the module doc for the exact formulas this backs. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct CostAnnotation { + pub value: Option, + pub unit: CostUnit, + pub source: CostSource, + #[serde(skip_serializing_if = "Option::is_none")] + pub baseline: Option, + /// `baseline_value - value` under `baseline`, when both are known — the + /// issue's `estimated_benefit_rate` (or its non-rate structural + /// analogue; see the module doc). + #[serde(skip_serializing_if = "Option::is_none")] + pub delta: Option, + /// `delta / baseline_value`, when `baseline_value > 0` — the issue's + /// `estimated_benefit_ratio`. Not part of the issue's own sketch schema + /// verbatim (the issue only asks that ratio be *derivable*), but kept + /// as its own explicit field rather than pushed onto the caller to + /// recompute from `delta` plus a `baseline` it would otherwise have no + /// value for. + #[serde(skip_serializing_if = "Option::is_none")] + pub benefit_ratio: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub model_version: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub benchmark_id: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub inputs: Vec, +} + +impl CostAnnotation { + /// No value at all — `value: None`, `source: Unavailable`. `unit` is + /// still required: it states what unit a value *would* have been in, + /// had one been available, so a renderer can group "not estimated" + /// alongside the kind of number it's missing. + pub fn unavailable(unit: CostUnit) -> Self { + CostAnnotation { + value: None, + unit, + source: CostSource::Unavailable, + baseline: None, + delta: None, + benefit_ratio: None, + model_version: None, + benchmark_id: None, + inputs: Vec::new(), + } + } + + /// A modeled value with no baseline comparison attached yet — chain + /// [`with_baseline`](Self::with_baseline) to add one. + pub fn modeled( + value: f64, + unit: CostUnit, + model_version: impl Into, + inputs: Vec, + ) -> Self { + CostAnnotation { + value: Some(value), + unit, + source: CostSource::Modeled, + baseline: None, + delta: None, + benefit_ratio: None, + model_version: Some(model_version.into()), + benchmark_id: None, + inputs, + } + } + + /// Attach `baseline` plus its own value, computing `delta` and + /// [`benefit_ratio`](Self::benefit_ratio) per the issue's formulas. + /// `self.value` must already be `Some` (an + /// [`unavailable`](Self::unavailable) annotation has nothing to + /// subtract a baseline from and is returned unchanged). + pub fn with_baseline(mut self, baseline: BaselineRef, baseline_value: f64) -> Self { + let Some(value) = self.value else { + return self; + }; + let delta = baseline_value - value; + self.delta = Some(delta); + self.benefit_ratio = benefit_ratio(baseline_value, delta); + self.baseline = Some(baseline); + self + } +} + +/// `delta / baseline_value`, or `None` when `baseline_value <= 0` — the +/// issue's explicit "ratio unavailable" guard (a zero or negative baseline +/// makes a ratio meaningless, not just numerically awkward). +pub fn benefit_ratio(baseline_value: f64, delta: f64) -> Option { + if baseline_value > 0.0 { + Some(delta / baseline_value) + } else { + None + } +} + +/// `total_cost(H) = recurring_cost_rate * H + one_shot_cost` — finite-run or +/// one-shot totals require an explicit horizon. Returns `None` (never a +/// fabricated total) when both terms are `None`, or when `horizon` isn't a +/// finite, non-negative number for a `Some(rate)`. Rate and one-shot costs +/// are passed as separate arguments specifically so they can never be +/// silently added by a caller before this function ever sees them. +pub fn total_cost(recurring_cost_rate: Option, horizon: f64, one_shot_cost: Option) -> Option { + let recurring = match recurring_cost_rate { + Some(rate) => { + if !horizon.is_finite() || horizon < 0.0 { + return None; + } + rate * horizon + } + None => 0.0, + }; + match (recurring_cost_rate, one_shot_cost) { + (None, None) => None, + _ => Some(recurring + one_shot_cost.unwrap_or(0.0)), + } +} + +/// Two [`CostAnnotation`]s were summed by [`sum_workload_costs`] despite +/// disagreeing on [`CostUnit`] — unit-incompatible aggregation is rejected +/// rather than silently mixed. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct UnitMismatch { + pub first: CostUnit, + pub second: CostUnit, +} + +impl std::fmt::Display for UnitMismatch { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "cannot aggregate cost annotations with mismatched units: {} vs {}", + self.first, self.second + ) + } +} +impl std::error::Error for UnitMismatch {} + +/// Sum a workload's per-node cost annotations into one workload-wide total, +/// counting each distinct `workload_node_id` exactly once — the "shared +/// nodes must be counted once in workload totals" requirement. `entries` is +/// `(workload_node_id, annotation)` pairs; an entry with `workload_node_id: +/// None` is never deduplicated against anything else (each is its own, +/// always-unique contribution). An entry whose `annotation.value` is `None` +/// (i.e. [`CostSource::Unavailable`]) contributes nothing to the sum but +/// does not invalidate it — the total is simply computed over whichever +/// inputs are actually known, same spirit as `Option`-typed inputs +/// elsewhere in this module. +/// +/// Rejects the sum with [`UnitMismatch`] the moment two *valued* entries +/// disagree on [`CostUnit`] — "unit-incompatible aggregation is rejected". +pub fn sum_workload_costs<'a, I>(entries: I) -> Result +where + I: IntoIterator, &'a CostAnnotation)>, +{ + let mut seen_ids = std::collections::HashSet::new(); + let mut unit: Option = None; + let mut total = 0.0_f64; + let mut counted_any = false; + let mut model_versions: Vec = Vec::new(); + + for (workload_node_id, annotation) in entries { + let Some(value) = annotation.value else { + continue; + }; + match unit { + None => unit = Some(annotation.unit), + Some(existing) if existing != annotation.unit => { + return Err(UnitMismatch { + first: existing, + second: annotation.unit, + }); + } + _ => {} + } + if let Some(id) = workload_node_id { + if !seen_ids.insert(id) { + continue; // already counted this shared node once + } + } + total += value; + counted_any = true; + if let Some(version) = &annotation.model_version { + if !model_versions.contains(version) { + model_versions.push(version.clone()); + } + } + } + + Ok(if counted_any { + CostAnnotation { + value: Some(total), + unit: unit.expect("counted_any implies unit was set"), + source: CostSource::Modeled, + baseline: None, + delta: None, + benefit_ratio: None, + model_version: if model_versions.len() == 1 { + Some(model_versions.remove(0)) + } else { + None + }, + benchmark_id: None, + inputs: Vec::new(), + } + } else { + CostAnnotation::unavailable(unit.unwrap_or(CostUnit::RelativeStructuralUnits)) + }) +} + +/// Whole-selected-workload cost/benefit — one query's (or one workload +/// batch's) aggregate baseline, selected, and benefit, built from +/// [`sum_workload_costs`] over that scope's own per-decision node +/// annotations. See [`crate::dag_export::NamedGraph::workload_cost`] / +/// [`crate::dag_export::WorkloadGraph::workload_cost`]. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct WorkloadCostSummary { + pub baseline_cost: CostAnnotation, + pub selected_cost: CostAnnotation, + pub benefit: CostAnnotation, +} + +/// Build a [`WorkloadCostSummary`] from one `(workload_node_id, baseline, +/// selected)` triple per decision node in scope — see +/// [`WorkloadCostSummary`]'s own doc. `model_version` labels the resulting +/// `benefit` annotation. +pub fn workload_cost_summary<'a, I>( + entries: I, + model_version: impl Into, +) -> Result +where + I: IntoIterator, &'a CostAnnotation, &'a CostAnnotation)> + Clone, +{ + let baseline_cost = sum_workload_costs(entries.clone().into_iter().map(|(id, baseline, _)| (id, baseline)))?; + let selected_cost = sum_workload_costs(entries.into_iter().map(|(id, _, selected)| (id, selected)))?; + + let benefit = match (baseline_cost.value, selected_cost.value) { + (Some(baseline_value), Some(selected_value)) if baseline_cost.unit == selected_cost.unit => { + let delta = baseline_value - selected_value; + CostAnnotation { + value: Some(delta), + unit: selected_cost.unit, + source: CostSource::Modeled, + baseline: Some(BaselineRef::PreAsapRecomputation), + delta: None, + benefit_ratio: benefit_ratio(baseline_value, delta), + model_version: Some(model_version.into()), + benchmark_id: None, + inputs: Vec::new(), + } + } + (Some(_), Some(_)) => { + return Err(UnitMismatch { + first: baseline_cost.unit, + second: selected_cost.unit, + }) + } + _ => CostAnnotation::unavailable(selected_cost.unit), + }; + + Ok(WorkloadCostSummary { + baseline_cost, + selected_cost, + benefit, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn unavailable_has_no_value_and_no_synthetic_number() { + let a = CostAnnotation::unavailable(CostUnit::RelativeStructuralUnits); + assert_eq!(a.value, None); + assert_eq!(a.source, CostSource::Unavailable); + } + + #[test] + fn with_baseline_computes_delta_and_ratio() { + let a = CostAnnotation::modeled(3.0, CostUnit::RelativeStructuralUnits, "v1", vec![]) + .with_baseline(BaselineRef::PreAsapRecomputation, 10.0); + assert_eq!(a.delta, Some(7.0)); + assert_eq!(a.benefit_ratio, Some(0.7)); + assert_eq!(a.baseline, Some(BaselineRef::PreAsapRecomputation)); + } + + #[test] + fn benefit_ratio_unavailable_when_baseline_not_positive() { + assert_eq!(benefit_ratio(0.0, 5.0), None); + assert_eq!(benefit_ratio(-1.0, 5.0), None); + assert_eq!(benefit_ratio(2.0, 1.0), Some(0.5)); + } + + #[test] + fn with_baseline_on_unavailable_annotation_is_a_no_op() { + let a = CostAnnotation::unavailable(CostUnit::CostUnits) + .with_baseline(BaselineRef::PreAsapRecomputation, 10.0); + assert_eq!(a.value, None); + assert_eq!(a.delta, None); + assert_eq!(a.baseline, None); + } + + #[test] + fn total_cost_requires_a_horizon_for_a_recurring_rate() { + assert_eq!(total_cost(Some(2.0), 5.0, None), Some(10.0)); + assert_eq!(total_cost(Some(2.0), 5.0, Some(1.0)), Some(11.0)); + assert_eq!(total_cost(None, 5.0, Some(4.0)), Some(4.0)); + assert_eq!(total_cost(None, 5.0, None), None); + } + + #[test] + fn total_cost_rejects_a_non_finite_or_negative_horizon() { + assert_eq!(total_cost(Some(2.0), f64::NAN, None), None); + assert_eq!(total_cost(Some(2.0), f64::INFINITY, None), None); + assert_eq!(total_cost(Some(2.0), -1.0, None), None); + } + + fn ann(value: f64, unit: CostUnit) -> CostAnnotation { + CostAnnotation::modeled(value, unit, "v1", vec![]) + } + + #[test] + fn sum_workload_costs_counts_a_shared_node_once() { + let a = ann(5.0, CostUnit::RelativeStructuralUnits); + let b = ann(5.0, CostUnit::RelativeStructuralUnits); + let c = ann(2.0, CostUnit::RelativeStructuralUnits); + // Node id 1 shared by two queries (same decision, same cost) must + // only be counted once; node id 2 is a distinct contribution. + let total = sum_workload_costs(vec![(Some(1), &a), (Some(1), &b), (Some(2), &c)]).unwrap(); + assert_eq!(total.value, Some(7.0), "5.0 (once) + 2.0, not 5+5+2"); + } + + #[test] + fn sum_workload_costs_never_deduplicates_entries_with_no_workload_id() { + let a = ann(3.0, CostUnit::RelativeStructuralUnits); + let b = ann(3.0, CostUnit::RelativeStructuralUnits); + let total = sum_workload_costs(vec![(None, &a), (None, &b)]).unwrap(); + assert_eq!(total.value, Some(6.0)); + } + + #[test] + fn sum_workload_costs_skips_unavailable_entries_without_failing() { + let known = ann(4.0, CostUnit::RelativeStructuralUnits); + let unavailable = CostAnnotation::unavailable(CostUnit::RelativeStructuralUnits); + let total = sum_workload_costs(vec![(None, &known), (None, &unavailable)]).unwrap(); + assert_eq!(total.value, Some(4.0)); + } + + #[test] + fn sum_workload_costs_rejects_mismatched_units() { + let rate = ann(1.0, CostUnit::CostUnitsPerSecond); + let structural = ann(1.0, CostUnit::RelativeStructuralUnits); + let err = sum_workload_costs(vec![(None, &rate), (None, &structural)]).unwrap_err(); + assert_eq!(err.first, CostUnit::CostUnitsPerSecond); + assert_eq!(err.second, CostUnit::RelativeStructuralUnits); + } + + #[test] + fn workload_cost_summary_computes_benefit_from_deduplicated_totals() { + let baseline1 = ann(10.0, CostUnit::RelativeStructuralUnits); + let selected1 = ann(3.0, CostUnit::RelativeStructuralUnits); + let baseline2 = ann(10.0, CostUnit::RelativeStructuralUnits); // same shared node + let selected2 = ann(3.0, CostUnit::RelativeStructuralUnits); + let baseline3 = ann(4.0, CostUnit::RelativeStructuralUnits); + let selected3 = ann(1.0, CostUnit::RelativeStructuralUnits); + + let entries = vec![ + (Some(1_u32), &baseline1, &selected1), + (Some(1_u32), &baseline2, &selected2), // duplicate of node 1 — must not double count + (Some(2_u32), &baseline3, &selected3), + ]; + let summary = workload_cost_summary(entries, "test-v1").unwrap(); + assert_eq!(summary.baseline_cost.value, Some(14.0), "10 (once) + 4"); + assert_eq!(summary.selected_cost.value, Some(4.0), "3 (once) + 1"); + assert_eq!(summary.benefit.value, Some(10.0)); + assert_eq!(summary.benefit.benefit_ratio, Some(10.0 / 14.0)); + } +} diff --git a/crates/types/src/dag_export.rs b/crates/types/src/dag_export.rs index 252d319..4048699 100644 --- a/crates/types/src/dag_export.rs +++ b/crates/types/src/dag_export.rs @@ -47,7 +47,8 @@ use std::rc::Rc; use serde::Serialize; use crate::post_asap::{SummaryExpr, SummaryNode}; -use crate::pre_asap::cse::{structural_hash, HashCache}; +use crate::cost::{CostAnnotation, CostInput, CostUnit}; +use crate::pre_asap::cse::{dag_node_count, structural_hash, HashCache}; use crate::pre_asap::query_expr::{QueryExpr, Source}; /// One flattened IR node. `detail` holds this node's own scalar fields @@ -152,6 +153,41 @@ pub struct DagDecision { /// `replacement_root` for the node replacing the pre-ASAP target; /// `replacement_region` for its generated or carried descendants. pub role: &'static str, + /// Structured counterpart of `cost` above — see [`CostAnnotation`] + /// (issue #286). `None` for the same reason `cost` can be `f64::NAN`: + /// the plugged-in cost model doesn't estimate a number for this + /// candidate shape. Additive: every existing reader of `cost` keeps + /// working unchanged; a reader that wants units, provenance, and an + /// explicit baseline comparison reads this instead. + #[serde(skip_serializing_if = "Option::is_none")] + pub baseline_cost: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub selected_cost: Option, + /// `baseline_cost.value - selected_cost.value` under `baseline_cost`'s + /// own baseline — for a winning `SharedSubtreeStrategy`/`CseShare` + /// decision this *is* "avoided recomputation for a shared sub-DAG" (one + /// of `dag_export`'s issue #286 granularity items): the baseline is + /// exactly the cost of recomputing this subtree independently at every + /// consumer, so the benefit is exactly what sharing avoided. + #[serde(skip_serializing_if = "Option::is_none")] + pub benefit: Option, +} + +/// A cost/benefit annotation attributed to one specific graph edge (`from` +/// -> `to`, in [`DagNode::children`]'s direction) rather than to a node — +/// issue #286's "edge cost only when genuinely attributable to the edge" +/// granularity item. `dag_export`'s own producer +/// ([`crates/devtools/src/bin/dag_export.rs`](../../../devtools/src/bin/dag_export.rs)) +/// only ever populates this for a `post_graph` edge whose target node is a +/// genuine DAG merge point (in-degree > 1, from +/// `deduplicate_pointer_shared_nodes`) — the materialization/read cost of +/// consuming an already-shared result along that one specific edge, never a +/// guessed multi-hop path cost (explicitly out of scope per the issue). +#[derive(Debug, Clone, Serialize)] +pub struct EdgeCostAnnotation { + pub from: u32, + pub to: u32, + pub cost: CostAnnotation, } /// One query's exported graph. `nodes[root as usize]` is the tree's root. @@ -159,6 +195,13 @@ pub struct DagDecision { pub struct DagGraph { pub nodes: Vec, pub root: u32, + /// See [`EdgeCostAnnotation`]. Always empty unless a higher layer + /// explicitly populated it (same layering rule as [`DagNode::notes`]); + /// omitted from JSON entirely when empty, so every existing producer of + /// [`DagGraph`] (every call to [`export`]/[`export_summary`]) is + /// unaffected. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub edge_annotations: Vec, } /// A single named query within a multi-query export. @@ -199,6 +242,17 @@ pub struct NamedGraph { /// `NamedGraph` is unaffected. #[serde(default, skip_serializing_if = "Option::is_none")] pub post_graph: Option, + /// This query's whole selected-workload cost/benefit — one of issue + /// #286's granularity items. Built by summing this query's own + /// `post_graph` decision-node cost annotations, deduplicated by + /// `workload_node_id` (a node this query shares with an earlier query + /// in the same export is still counted once here, since + /// `assign_workload_node_ids` assigns identity workload-wide, not + /// per-query). `None` unless a higher layer built one (same + /// `--post-asap`-gated pattern as `post_graph`); omitted from JSON when + /// absent. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub workload_cost: Option, } /// A batch of named queries — the shape the viewer's multi-query / compare @@ -208,6 +262,14 @@ pub struct NamedGraph { #[derive(Debug, Clone, Serialize)] pub struct WorkloadGraph { pub queries: Vec, + /// The selected multi-query workload's own cost/benefit, deduplicated + /// across every query in `queries` (not just within one) — the + /// "Selecting ... multiple queries ... display correct Pre/Post-ASAP + /// annotations" / "workload totals count shared nodes once" acceptance + /// criteria for the batch/union case. `None` unless a higher layer + /// built one; omitted from JSON when absent. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub workload_cost: Option, } // ── Post-ASAP replacement export — a second, layering-seam-shaped feature ── @@ -484,6 +546,21 @@ pub struct TargetReplacement { /// `export(target)` for the `MemoGroup`'s own `target`, reused as-is. pub before: DagGraph, pub after: TargetReplacementAfter, + /// Structured baseline/selected/benefit cost annotations for this one + /// replacement region — issue #286's "replacement-region baseline + /// cost, selected cost, and benefit" granularity item. Always + /// consistent with `cost` above: `selected_cost.value == Some(cost)` + /// whenever `cost` is finite, `None`/`Unavailable` whenever it's + /// `NaN`. `baseline_cost` is always populated (the pre-ASAP + /// independent-recomputation baseline is computable from the target + /// alone, unlike a candidate's own estimated cost); `benefit` is + /// `Unavailable` exactly when `selected_cost` is. + #[serde(skip_serializing_if = "Option::is_none")] + pub baseline_cost: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub selected_cost: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub benefit: Option, } /// What a [`TargetReplacement`] became — either a genuine post-ASAP binding @@ -520,7 +597,11 @@ pub fn export(expr: &QueryExpr) -> DagGraph { // callback regardless (so `export_post_asap` can share this exact // per-variant traversal instead of duplicating it). let root = build(expr, &mut nodes, &mut cache, &mut |_| None); - DagGraph { nodes, root } + DagGraph { + nodes, + root, + edge_annotations: Vec::new(), + } } /// What a higher layer found for one specific pre-ASAP node when building a @@ -623,12 +704,73 @@ fn deduplicate_pointer_shared_nodes(nodes: Vec, root: u32) -> DagGraph deduplicated.push(node); } + let edge_annotations = shared_node_edge_annotations(&deduplicated); DagGraph { nodes: deduplicated, root: old_to_new[root as usize], + edge_annotations, } } +/// One [`EdgeCostAnnotation`] per edge running *into* a genuine DAG merge +/// point in `nodes` — a node id referenced as a child by more than one +/// parent, the exact shape [`deduplicate_pointer_shared_nodes`] produces +/// for a subtree two or more consumers share by `Rc` pointer identity +/// (whether or not a `SharedSubtreeStrategy` decision is what caused it — +/// ordinary `Rc`-shared pre-ASAP structure merges here too). This is the +/// "edge cost only when genuinely attributable to the edge" granularity +/// item from issue #286: the materialization/read cost of one consumer +/// reading the already-computed shared result along its own specific edge, +/// evenly divided across every consuming edge — never a guessed multi-hop +/// path cost (explicitly out of scope per the issue). +/// +/// Uses [`dag_node_count`] (the same structural-size proxy +/// `asap_aware_mapping::cost_model::default_cse_recompute_cost` computes; +/// duplicated here in plain terms since `asap_types` may not depend on that +/// higher crate) rather than a real per-byte transfer cost — honestly +/// unit-tagged [`CostUnit::RelativeStructuralUnits`], not a rate. +fn shared_node_edge_annotations(nodes: &[DagNode]) -> Vec { + let mut parents_of: HashMap> = HashMap::new(); + for node in nodes { + for &child in &node.children { + parents_of.entry(child).or_default().push(node.id); + } + } + + let mut annotations = Vec::new(); + for (child_id, parents) in &parents_of { + if parents.len() < 2 { + continue; + } + let Some(child) = nodes.get(*child_id as usize) else { + continue; + }; + let Some(source_expr) = child.source_expr.as_ref() else { + continue; // no QueryExpr to size (e.g. a post-ASAP-originated node) + }; + let consumer_count = parents.len(); + let materialized_size = dag_node_count(source_expr) as f64; + let per_edge_cost = materialized_size / consumer_count as f64; + for &parent in parents { + annotations.push(EdgeCostAnnotation { + from: *child_id, + to: parent, + cost: CostAnnotation::modeled( + per_edge_cost, + CostUnit::RelativeStructuralUnits, + "dag_export-shared-edge-v1", + vec![ + CostInput::new("materialized_subtree_size", materialized_size), + CostInput::new("consumer_count", consumer_count as f64), + ], + ), + }); + } + } + annotations.sort_by_key(|edge| (edge.from, edge.to)); + annotations +} + /// Push one flattened node for `expr`. `expr` is the *whole* subtree this /// node represents (not just its own fields) — `hash` is /// [`structural_hash(expr)`](structural_hash), the identical function and @@ -1251,6 +1393,7 @@ mod tests { assert!(graph.nodes[0].notes.is_empty()); assert!(graph.nodes[0].decision.is_none()); assert!(graph.nodes[0].schema.is_some()); + assert!(graph.edge_annotations.is_empty()); let json = serde_json::to_string(&graph.nodes[0]).unwrap(); assert!( !json.contains("notes"), @@ -1260,6 +1403,64 @@ mod tests { !json.contains("decision"), "empty `decision` must be skipped, not serialized as `null`: {json}" ); + let graph_json = serde_json::to_string(&graph).unwrap(); + assert!( + !graph_json.contains("edge_annotations"), + "empty `edge_annotations` must be skipped, not serialized as `[]`: {graph_json}" + ); + } + + // ── Issue #286: edge cost annotations for genuine DAG merge points ──── + + #[test] + fn export_post_asap_annotates_edges_into_a_genuinely_shared_node() { + // Two parents (a Join's own two sides) share the exact same `Rc` + // Scan — `export_post_asap`'s `deduplicate_pointer_shared_nodes` + // must merge them onto one node id, and (issue #286) attach an + // `EdgeCostAnnotation` on each of the two edges running into it. + let shared_scan = Rc::new(scan("metrics", value_col())); + let root = QueryExpr::Join { + kind: crate::pre_asap::query_expr::JoinKind::Inner, + pred: Predicate(Rc::new(QueryExpr::Literal(ScalarValue::Boolean(true)))), + left: Rc::clone(&shared_scan), + right: Rc::clone(&shared_scan), + }; + let graph = export_post_asap(&root, &mut |_| None); + + assert_eq!( + graph.nodes.iter().filter(|n| n.kind == "Scan").count(), + 1, + "the shared Scan must be merged onto one node, not duplicated" + ); + let scan_id = graph.nodes.iter().find(|n| n.kind == "Scan").unwrap().id; + let edges_into_scan: Vec<_> = graph + .edge_annotations + .iter() + .filter(|edge| edge.from == scan_id) + .collect(); + assert_eq!(edges_into_scan.len(), 2, "one annotation per consuming edge"); + for edge in &edges_into_scan { + assert_eq!(edge.cost.value, Some(0.5), "1 unique node / 2 consumers"); + assert_eq!(edge.cost.unit, crate::cost::CostUnit::RelativeStructuralUnits); + assert_eq!(edge.cost.source, crate::cost::CostSource::Modeled); + } + } + + #[test] + fn export_never_produces_edge_annotations_since_it_never_shares_nodes() { + // Plain `export` (no `export_post_asap`) never deduplicates by `Rc` + // pointer identity — even a workload-level shared subtree renders as + // two independent tree nodes here, so there is nothing to annotate. + let shared_scan = Rc::new(scan("metrics", value_col())); + let root = QueryExpr::Join { + kind: crate::pre_asap::query_expr::JoinKind::Inner, + pred: Predicate(Rc::new(QueryExpr::Literal(ScalarValue::Boolean(true)))), + left: Rc::clone(&shared_scan), + right: Rc::clone(&shared_scan), + }; + let graph = export(&root); + assert_eq!(graph.nodes.iter().filter(|n| n.kind == "Scan").count(), 2); + assert!(graph.edge_annotations.is_empty()); } #[test] diff --git a/crates/types/src/lib.rs b/crates/types/src/lib.rs index 1308b97..dbc1665 100644 --- a/crates/types/src/lib.rs +++ b/crates/types/src/lib.rs @@ -19,6 +19,7 @@ //! runtime's readout path can call directly — see that module's docs //! for the planning-time/execution-time boundary and why it's unwired //! today. +pub mod cost; pub mod dag_export; pub mod post_asap; pub mod pre_asap; diff --git a/tools/dag-viewer/README.md b/tools/dag-viewer/README.md index 1c3fd69..20c5bd2 100644 --- a/tools/dag-viewer/README.md +++ b/tools/dag-viewer/README.md @@ -13,6 +13,11 @@ The viewer has one visualization mode: **Pre/Post-ASAP**. target operation derives its output schema in the details panel. - The details panel shows the selected workload's bound table/metric schemas and can be resized by dragging its left edge. +- A post-ASAP node whose winning decision carries a cost/benefit annotation + shows a concise `▼NN%`/`▲NN%` badge next to its label; the sidebar and the + workload-scope summary show the full baseline/selected/benefit breakdown, + with units and provenance, wherever the export provides one — see "Cost/ + benefit annotations" below. There are no separate Single, Compare, or Union modes. @@ -82,7 +87,10 @@ a selected replacement directly contains: "rationale": "count realizes as a Cms sketch", "rank": 0, "cost": 3.0, - "role": "replacement_root" + "role": "replacement_root", + "baseline_cost": { "value": 6.0, "unit": "RelativeStructuralUnits", "source": "Modeled", "model_version": "dag_export-structural-cost-v1" }, + "selected_cost": { "value": 3.0, "unit": "RelativeStructuralUnits", "source": "Modeled", "baseline": {"kind": "PreAsapRecomputation"}, "delta": 3.0, "benefit_ratio": 0.5 }, + "benefit": { "value": 3.0, "unit": "RelativeStructuralUnits", "source": "Modeled", "baseline": {"kind": "PreAsapRecomputation"}, "benefit_ratio": 0.5 } } } ``` @@ -97,6 +105,34 @@ filter predicates, projections, sources, summary families, and readout queries. Category icons are deliberately omitted so they cannot be confused with IR text. +### Cost/benefit annotations (issue #286) + +`decision.baseline_cost` / `.selected_cost` / `.benefit` are structured +[`CostAnnotation`](../../crates/types/src/cost.rs)s: `value` + `unit` + +`source` (`Modeled` / `Measured` / `Unavailable`), optionally `baseline` + +`delta` + `benefit_ratio`, and `model_version`/`benchmark_id`/`inputs` for +provenance. A missing `value` (`source: "Unavailable"`) always renders as +**Not estimated** — the viewer never fabricates a number. Today every value +`dag_export` produces is unit-tagged `RelativeStructuralUnits`: a +structural-size proxy (the same one `asap_aware_mapping::cost_model` +already uses for ranking), not a real cost-per-second rate — issue #287's +recurrence-aware inputs (`update_rate`/`evaluation_rate`/`query_interval`) +are what would let a future export use `CostUnitsPerSecond` instead; the +annotation plumbing already accepts that unit unchanged. + +The same three fields also appear on `TargetReplacement` +(replacement-region baseline/selected/benefit), `NamedGraph.workload_cost` / +`WorkloadGraph.workload_cost` (whole selected-workload cost/benefit, shared +nodes counted once via `decision.id` dedup), and `DagGraph.edge_annotations` +(materialization/read cost on an edge into a genuine DAG merge point — never +a guessed multi-hop path cost). The sidebar shows the full breakdown +(value, unit, provenance, baseline, ratio, inputs) on node/edge click and in +the workload-scope summary; a post-ASAP node with a costed decision also +gets a concise on-graph `▼NN%`/`▲NN%` badge next to its label. + +All of this is additive and optional: an export with none of these fields +(anything produced before issue #286) renders exactly as before. + ## Tests ```sh diff --git a/tools/dag-viewer/dag.example.json b/tools/dag-viewer/dag.example.json index 401aa0a..de28c8f 100644 --- a/tools/dag-viewer/dag.example.json +++ b/tools/dag-viewer/dag.example.json @@ -497,6 +497,43 @@ ], "root": 2 } + }, + "baseline_cost": { + "value": 2.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 2.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 3.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.5, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.5, + "model_version": "dag_export-structural-cost-v1" } } ], @@ -597,7 +634,44 @@ "rationale": "count realizes as a Cms sketch", "rank": 0, "cost": 3.0, - "role": "replacement_region" + "role": "replacement_region", + "baseline_cost": { + "value": 2.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 2.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 3.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.5, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.5, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -639,7 +713,44 @@ "rationale": "count realizes as a Cms sketch", "rank": 0, "cost": 3.0, - "role": "replacement_region" + "role": "replacement_region", + "baseline_cost": { + "value": 2.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 2.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 3.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.5, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.5, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -674,7 +785,44 @@ "rationale": "count realizes as a Cms sketch", "rank": 0, "cost": 3.0, - "role": "replacement_root" + "role": "replacement_root", + "baseline_cost": { + "value": 2.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 2.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 3.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.5, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.5, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -729,6 +877,30 @@ } ], "root": 3 + }, + "workload_cost": { + "baseline_cost": { + "value": 2.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1" + }, + "selected_cost": { + "value": 3.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.5, + "model_version": "dag_export-workload-cost-v1" + } } }, { @@ -1451,6 +1623,43 @@ ], "root": 5 } + }, + "baseline_cost": { + "value": 2.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 2.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 2.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": 0.0, + "benefit_ratio": 0.0, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": 0.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": 0.0, + "model_version": "dag_export-structural-cost-v1" } } ], @@ -1551,7 +1760,44 @@ "rationale": "Sum { col: Some(3) } realizes as an exact Sum accumulator", "rank": 0, "cost": 3.0, - "role": "replacement_region" + "role": "replacement_region", + "baseline_cost": { + "value": 2.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 2.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 3.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.5, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.5, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -1598,7 +1844,44 @@ "rationale": "Sum { col: Some(3) } realizes as an exact Sum accumulator", "rank": 0, "cost": 3.0, - "role": "replacement_root" + "role": "replacement_root", + "baseline_cost": { + "value": 2.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 2.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 3.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.5, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.5, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -1662,7 +1945,44 @@ "rationale": "Rewrites AVG into SUM and COUNT under the same grouping, then divides SUM by COUNT.", "rank": 0, "cost": 2.0, - "role": "replacement_region" + "role": "replacement_region", + "baseline_cost": { + "value": 2.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 2.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 2.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": 0.0, + "benefit_ratio": 0.0, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": 0.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": 0.0, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -1704,7 +2024,44 @@ "rationale": "count realizes as an exact Count accumulator", "rank": 0, "cost": 3.0, - "role": "replacement_root" + "role": "replacement_root", + "baseline_cost": { + "value": 2.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 2.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 3.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.5, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.5, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -1750,7 +2107,44 @@ "rationale": "Rewrites AVG into SUM and COUNT under the same grouping, then divides SUM by COUNT.", "rank": 0, "cost": 2.0, - "role": "replacement_root" + "role": "replacement_root", + "baseline_cost": { + "value": 2.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 2.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 2.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": 0.0, + "benefit_ratio": 0.0, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": 0.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": 0.0, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -1804,7 +2198,73 @@ "hash": 5277207484379927121 } ], - "root": 5 + "root": 5, + "edge_annotations": [ + { + "from": 0, + "to": 1, + "cost": { + "value": 0.5, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-shared-edge-v1", + "inputs": [ + { + "name": "materialized_subtree_size", + "value": 1.0 + }, + { + "name": "consumer_count", + "value": 2.0 + } + ] + } + }, + { + "from": 0, + "to": 3, + "cost": { + "value": 0.5, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-shared-edge-v1", + "inputs": [ + { + "name": "materialized_subtree_size", + "value": 1.0 + }, + { + "name": "consumer_count", + "value": 2.0 + } + ] + } + } + ] + }, + "workload_cost": { + "baseline_cost": { + "value": 6.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1" + }, + "selected_cost": { + "value": 8.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -2.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-workload-cost-v1" + } } }, { @@ -2280,6 +2740,43 @@ ], "root": 1 } + }, + "baseline_cost": { + "value": 3.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 3.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 4.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-structural-cost-v1" } }, { @@ -2731,6 +3228,43 @@ ], "root": 5 } + }, + "baseline_cost": { + "value": 5.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 5.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 5.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": 0.0, + "benefit_ratio": 0.0, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": 0.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": 0.0, + "model_version": "dag_export-structural-cost-v1" } } ], @@ -2795,7 +3329,44 @@ "rationale": "Rate realizes as an exact Rate accumulator", "rank": 0, "cost": 4.0, - "role": "replacement_region" + "role": "replacement_region", + "baseline_cost": { + "value": 3.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 3.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 4.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -2838,7 +3409,44 @@ "rationale": "Rate realizes as an exact Rate accumulator", "rank": 0, "cost": 4.0, - "role": "replacement_region" + "role": "replacement_region", + "baseline_cost": { + "value": 3.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 3.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 4.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -2876,7 +3484,44 @@ "rationale": "Rate realizes as an exact Rate accumulator", "rank": 0, "cost": 4.0, - "role": "replacement_root" + "role": "replacement_root", + "baseline_cost": { + "value": 3.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 3.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 4.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -2925,7 +3570,44 @@ "rationale": "Derives this smaller top-k from a compatible larger top-k result shared by the workload.", "rank": 0, "cost": 5.0, - "role": "replacement_region" + "role": "replacement_region", + "baseline_cost": { + "value": 5.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 5.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 5.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": 0.0, + "benefit_ratio": 0.0, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": 0.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": 0.0, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -2966,7 +3648,44 @@ "rationale": "Derives this smaller top-k from a compatible larger top-k result shared by the workload.", "rank": 0, "cost": 5.0, - "role": "replacement_region" + "role": "replacement_region", + "baseline_cost": { + "value": 5.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 5.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 5.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": 0.0, + "benefit_ratio": 0.0, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": 0.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": 0.0, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -3007,11 +3726,72 @@ "rationale": "Derives this smaller top-k from a compatible larger top-k result shared by the workload.", "rank": 0, "cost": 5.0, - "role": "replacement_root" + "role": "replacement_root", + "baseline_cost": { + "value": 5.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 5.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 5.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": 0.0, + "benefit_ratio": 0.0, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": 0.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": 0.0, + "model_version": "dag_export-structural-cost-v1" + } } } ], "root": 5 + }, + "workload_cost": { + "baseline_cost": { + "value": 8.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1" + }, + "selected_cost": { + "value": 9.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.125, + "model_version": "dag_export-workload-cost-v1" + } } }, { @@ -3487,6 +4267,43 @@ ], "root": 1 } + }, + "baseline_cost": { + "value": 3.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 3.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 4.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-structural-cost-v1" } } ], @@ -3551,7 +4368,44 @@ "rationale": "Rate realizes as an exact Rate accumulator", "rank": 0, "cost": 4.0, - "role": "replacement_region" + "role": "replacement_region", + "baseline_cost": { + "value": 3.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 3.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 4.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -3594,7 +4448,44 @@ "rationale": "Rate realizes as an exact Rate accumulator", "rank": 0, "cost": 4.0, - "role": "replacement_region" + "role": "replacement_region", + "baseline_cost": { + "value": 3.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 3.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 4.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -3632,7 +4523,44 @@ "rationale": "Rate realizes as an exact Rate accumulator", "rank": 0, "cost": 4.0, - "role": "replacement_root" + "role": "replacement_root", + "baseline_cost": { + "value": 3.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 3.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 4.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -3711,6 +4639,30 @@ } ], "root": 4 + }, + "workload_cost": { + "baseline_cost": { + "value": 3.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1" + }, + "selected_cost": { + "value": 4.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.3333333333333333, + "model_version": "dag_export-workload-cost-v1" + } } }, { @@ -4590,6 +5542,43 @@ ], "root": 2 } + }, + "baseline_cost": { + "value": 4.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 4.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 5.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.25, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.25, + "model_version": "dag_export-structural-cost-v1" } } ], @@ -4690,7 +5679,44 @@ "rationale": "count realizes as a Cms sketch", "rank": 0, "cost": 5.0, - "role": "replacement_region" + "role": "replacement_region", + "baseline_cost": { + "value": 4.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 4.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 5.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.25, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.25, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -4752,7 +5778,44 @@ "rationale": "count realizes as a Cms sketch", "rank": 0, "cost": 5.0, - "role": "replacement_region" + "role": "replacement_region", + "baseline_cost": { + "value": 4.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 4.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 5.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.25, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.25, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -4834,7 +5897,44 @@ "rationale": "count realizes as a Cms sketch", "rank": 0, "cost": 5.0, - "role": "replacement_region" + "role": "replacement_region", + "baseline_cost": { + "value": 4.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 4.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 5.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.25, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.25, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -4876,7 +5976,44 @@ "rationale": "count realizes as a Cms sketch", "rank": 0, "cost": 5.0, - "role": "replacement_region" + "role": "replacement_region", + "baseline_cost": { + "value": 4.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 4.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 5.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.25, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.25, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -4911,7 +6048,44 @@ "rationale": "count realizes as a Cms sketch", "rank": 0, "cost": 5.0, - "role": "replacement_root" + "role": "replacement_root", + "baseline_cost": { + "value": 4.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1", + "inputs": [ + { + "name": "per_consumer_recompute_cost", + "value": 4.0 + }, + { + "name": "consumer_count", + "value": 1.0 + } + ] + }, + "selected_cost": { + "value": 5.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "delta": -1.0, + "benefit_ratio": -0.25, + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.25, + "model_version": "dag_export-structural-cost-v1" + } } }, { @@ -4966,7 +6140,55 @@ } ], "root": 5 + }, + "workload_cost": { + "baseline_cost": { + "value": 4.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1" + }, + "selected_cost": { + "value": 5.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -1.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.25, + "model_version": "dag_export-workload-cost-v1" + } } } - ] + ], + "workload_cost": { + "baseline_cost": { + "value": 20.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1" + }, + "selected_cost": { + "value": 25.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "model_version": "dag_export-structural-cost-v1" + }, + "benefit": { + "value": -5.0, + "unit": "RelativeStructuralUnits", + "source": "Modeled", + "baseline": { + "kind": "PreAsapRecomputation" + }, + "benefit_ratio": -0.25, + "model_version": "dag_export-workload-cost-v1" + } + } } diff --git a/tools/dag-viewer/index.html b/tools/dag-viewer/index.html index cb1a1d0..e324ebc 100644 --- a/tools/dag-viewer/index.html +++ b/tools/dag-viewer/index.html @@ -372,6 +372,40 @@ .leg .swatchLabel { font-weight: 650; } .leg .swatchDesc { color: var(--muted); display: block; } .leg .swatch.ring { background: transparent; border-radius: 999px; border-width: 2px; } + + /* Issue #286: structured cost/benefit annotations. */ + .costBlock { + margin: 0.5rem 0; + padding: 0.5rem 0.55rem; + border: 1px solid var(--border); + border-radius: 8px; + background: var(--panel2); + } + .costBlock h4 { + margin: 0 0 0.35rem; + font-size: 0.68rem; + text-transform: uppercase; + letter-spacing: .03em; + color: var(--muted); + } + .costRow { display: flex; align-items: baseline; gap: 0.4rem; flex-wrap: wrap; margin: 0.15rem 0; } + .costRow .costLabel { color: var(--muted); font-size: 0.72rem; min-width: 72px; } + .costRow .costValue { font-weight: 650; font-variant-numeric: tabular-nums; } + .costBadge { + display: inline-block; + font-size: 0.64rem; + font-weight: 700; + text-transform: uppercase; + letter-spacing: .02em; + border-radius: 4px; + padding: 0.05rem 0.35rem; + } + .costBadge--modeled { color: var(--accent); background: color-mix(in srgb, var(--accent) 16%, transparent); } + .costBadge--measured { color: #15803d; background: color-mix(in srgb, #15803d 16%, transparent); } + .costBadge--unavailable { color: var(--muted); background: color-mix(in srgb, var(--muted) 16%, transparent); } + .costMeta { color: var(--muted); font-size: 0.68rem; margin-top: 0.15rem; } + .costInputs { margin: 0.3rem 0 0; padding: 0; list-style: none; font-size: 0.68rem; color: var(--muted); } + .costInputs li { display: flex; justify-content: space-between; gap: 0.5rem; padding: 0.05rem 0; } diff --git a/tools/dag-viewer/render.py b/tools/dag-viewer/render.py index 2059597..486ed85 100755 --- a/tools/dag-viewer/render.py +++ b/tools/dag-viewer/render.py @@ -20,11 +20,12 @@ `QueryExpr` detail *is* its plan (see the side panel on node click), and shared-hash highlighting *is* what this repo has for CSE today — both a hash-based proxy, not real CSE output; see README.md's "Shared-subtree -highlighting is a proxy" section. Per-node cost isn't in dag_export's output -yet (no cost estimator is wired into pre-ASAP IR), so there's nothing here -to render for it; if `detail` ever grows a `cost` field, the side panel -shows it automatically since it dumps `detail` verbatim, no viewer change -needed. +highlighting is a proxy" section. Structured cost/benefit annotations +(issue #286, `CostAnnotation` in crates/types/src/cost.rs) pass through +this deep-copy untouched, same as everything else `prepare_workload` below +doesn't explicitly rewrite — viewer.js reads `decision.baseline_cost` / +`.selected_cost` / `.benefit`, `NamedGraph.workload_cost`, and +`DagGraph.edge_annotations` directly, with no help needed from this file. Usage: cargo run -p asap-devtools --bin dag_export -- --sql "..." --name q1 \\ diff --git a/tools/dag-viewer/viewer.js b/tools/dag-viewer/viewer.js index feca4f8..509acd9 100644 --- a/tools/dag-viewer/viewer.js +++ b/tools/dag-viewer/viewer.js @@ -506,6 +506,7 @@ function unionStageLaneElements(stage, chosen) { function laneElements(laneId, laneLabel, graph, query, stage) { const nodes = graph.nodes; const byId = new Map(nodes.map((node) => [node.id, node])); + const edgeCostByPair = new Map((graph.edge_annotations || []).map((edge) => [`${edge.from}${edge.to}`, edge.cost])); const elements = [ { data: { id: laneId, label: laneLabel, isLane: true }, classes: 'laneParent', selectable: false, grabbable: false }, ]; @@ -514,7 +515,11 @@ function laneElements(laneId, laneLabel, graph, query, stage) { data: { id: `${laneId}-${node.id}`, parent: laneId, - label: node.label, + // On-graph label carries a concise cost/benefit badge (issue #286) + // when this node's decision has one; `node.label` itself (nested, + // used everywhere else — the sidebar, schema derivation, …) stays + // exactly the plain IR label. + label: node.label + nodeCostBadgeSuffix(node), node, // Flat (not nested under `node`) so buildCyStyle's // `node[kind = "KeepPreAsap"]` selector can actually match it — @@ -541,6 +546,11 @@ function laneElements(laneId, laneLabel, graph, query, stage) { source: `${laneId}-${childId}`, target: `${laneId}-${node.id}`, schemaLabel: formatSchema(byId.get(childId).schema), + // Issue #286 edge cost — only ever present when the exporter + // found this exact (child -> node) edge genuinely attributable + // (a real DAG merge point); `undefined` otherwise, read by + // showEdgeDetail. + edgeCost: edgeCostByPair.get(`${childId} ${node.id}`), }, }); } @@ -548,6 +558,152 @@ function laneElements(laneId, laneLabel, graph, query, stage) { return elements; } +// ── Issue #286: structured cost/benefit annotations ─────────────────────── +// Renders only what a `dag_export` JSON export explicitly carries +// (`CostAnnotation`/`WorkloadCostSummary`/`EdgeCostAnnotation` from +// crates/types/src/cost.rs) — no client-side cost estimation. A value with +// no `source: "Modeled"|"Measured"` (i.e. `Unavailable`, or the field +// simply absent from an older export) always reads "Not estimated", never +// a fabricated number. + +function formatCostUnit(unit) { + switch (unit) { + case 'CostUnitsPerSecond': return 'cost units/s'; + case 'CostUnits': return 'cost units'; + case 'RelativeStructuralUnits': return 'relative structural units'; + default: return unit || 'unknown unit'; + } +} + +function formatBaselineRef(baseline) { + if (!baseline) return ''; + switch (baseline.kind) { + case 'PreAsapRecomputation': return 'pre-ASAP recomputation'; + case 'HighestRankedNonSelectedCandidate': + return `best non-selected candidate (rank ${baseline.detail && baseline.detail.rank})`; + case 'Named': return String(baseline.detail || ''); + default: return baseline.kind || ''; + } +} + +function formatCostNumber(value) { + // Trim to at most 3 decimals without trailing zeros — these are + // structural-proxy magnitudes today (see cost.rs's module doc), not + // precision-sensitive measurements. + return Number(value.toFixed(3)).toString(); +} + +// One `` block for the sidebar: value + unit + +// Modeled/Measured/Unavailable badge, baseline/delta/ratio when present, +// model/benchmark provenance, and the raw `inputs` the value was built +// from. `annotation` may be `undefined` (an older export with no +// annotation at all) or `null`/missing `value` (an explicit `Unavailable`) +// — both render as "Not estimated", never a number. +function renderCostAnnotation(title, annotation) { + if (!annotation) return ''; + const source = annotation.source || 'Unavailable'; + const badgeClass = source === 'Modeled' ? 'costBadge--modeled' : source === 'Measured' ? 'costBadge--measured' : 'costBadge--unavailable'; + if (annotation.value === null || annotation.value === undefined) { + return `
${escapeHtml(title)}Not estimated${escapeHtml(source)}
`; + } + const unit = formatCostUnit(annotation.unit); + const valueText = `${formatCostNumber(annotation.value)} ${unit}`; + const metaParts = []; + if (annotation.baseline) metaParts.push(`vs ${formatBaselineRef(annotation.baseline)}`); + if (typeof annotation.delta === 'number') metaParts.push(`Δ ${formatCostNumber(annotation.delta)} ${unit}`); + if (typeof annotation.benefit_ratio === 'number') metaParts.push(`ratio ${(annotation.benefit_ratio * 100).toFixed(1)}%`); + const provenanceParts = []; + if (annotation.model_version) provenanceParts.push(`model ${annotation.model_version}`); + if (annotation.benchmark_id) provenanceParts.push(`benchmark ${annotation.benchmark_id}`); + const inputsHtml = (annotation.inputs || []).length + ? `
    ${annotation.inputs.map((input) => `
  • ${escapeHtml(input.name)}${escapeHtml(String(input.value))}${input.unit ? ' ' + escapeHtml(input.unit) : ''}
    • `).join('')}
` + : ''; + return ` +
+ ${escapeHtml(title)} + ${escapeHtml(valueText)} + ${escapeHtml(source)} +
+ ${metaParts.length ? `
${escapeHtml(metaParts.join(' · '))}
` : ''} + ${provenanceParts.length ? `
${escapeHtml(provenanceParts.join(' · '))}
` : ''} + ${inputsHtml} + `; +} + +// Baseline/selected/benefit trio for one replacement decision, matching +// `DagDecision.baseline_cost/selected_cost/benefit` (crates/types/src/dag_export.rs). +function renderDecisionCostBlock(entry) { + if (!entry.baseline_cost && !entry.selected_cost && !entry.benefit) return ''; + return `
+

Cost / benefit

+ ${renderCostAnnotation('Baseline', entry.baseline_cost)} + ${renderCostAnnotation('Selected', entry.selected_cost)} + ${renderCostAnnotation('Benefit', entry.benefit)} +
`; +} + +// Short, on-graph badge text for a post-ASAP node's own winning decision — +// "concise on-graph benefit/cost badges" per issue #286; the full +// breakdown only ever appears in the sidebar (`renderDecisionCostBlock`). +// Empty string whenever there's nothing to show (no decision, or its +// benefit is `Unavailable`) so an un-costed node's label is untouched. +function nodeCostBadgeSuffix(node) { + const benefit = node.decision && node.decision.benefit; + if (!benefit || benefit.value === null || benefit.value === undefined) return ''; + if (typeof benefit.benefit_ratio === 'number') { + const pct = Math.abs(benefit.benefit_ratio * 100); + return `\n${benefit.benefit_ratio >= 0 ? '▼' : '▲'}${pct >= 10 ? Math.round(pct) : pct.toFixed(1)}%`; + } + return `\n${benefit.value >= 0 ? '▼' : '▲'}${formatCostNumber(Math.abs(benefit.value))}`; +} + +// Workload-wide baseline/selected/benefit for the currently selected +// queries, deduplicated by `decision.id` — the same collision-free key +// `crates/devtools/src/bin/dag_export.rs`'s own `decision_cost_entries` +// dedupes by (a decision spans every node in its replacement region, and a +// CSE-shared target can appear in more than one selected query). This +// aggregates explicit per-node `CostAnnotation`s already in the export; it +// never estimates a cost itself. Returns `null` when nothing in the +// selection carries a cost annotation, or when selected annotations +// disagree on unit (unit-incompatible aggregation is refused, not mixed). +function computeSelectionWorkloadCost(selected) { + const seenDecisions = new Set(); + let unit = null; + let baselineSum = 0; + let selectedSum = 0; + let any = false; + for (const query of selected) { + const nodes = (query.post_graph && query.post_graph.nodes) || []; + for (const node of nodes) { + const decision = node.decision; + if (!decision || seenDecisions.has(decision.id)) continue; + seenDecisions.add(decision.id); + const baseline = decision.baseline_cost; + const selectedCost = decision.selected_cost; + if (!baseline || !selectedCost) continue; + if (baseline.value === null || baseline.value === undefined || selectedCost.value === null || selectedCost.value === undefined) continue; + if (unit === null) unit = baseline.unit; + if (baseline.unit !== unit || selectedCost.unit !== unit) return null; // unit-incompatible aggregation is rejected + baselineSum += baseline.value; + selectedSum += selectedCost.value; + any = true; + } + } + if (!any) return null; + const delta = baselineSum - selectedSum; + return { + baseline_cost: { value: baselineSum, unit, source: 'Modeled' }, + selected_cost: { value: selectedSum, unit, source: 'Modeled' }, + benefit: { + value: delta, + unit, + source: 'Modeled', + baseline: { kind: 'PreAsapRecomputation' }, + benefit_ratio: baselineSum > 0 ? delta / baselineSum : null, + }, + }; +} + function formatSchema(schema) { if (!schema || typeof schema !== 'object') return 'schema unavailable'; const fields = Array.isArray(schema.columns) ? schema.columns : schema.fields; @@ -595,6 +751,10 @@ function showEdgeDetail(edge) { const sourceNode = source.data('node') || {}; const targetNode = target.data('node') || {}; const edgeSchema = edge.data('schemaLabel') || formatSchema(sourceNode.schema); + const edgeCost = edge.data('edgeCost'); + const edgeCostHtml = edgeCost + ? `

Edge cost

${renderCostAnnotation('Materialization', edgeCost)}
` + : ''; detailSection.innerHTML = `

Selected edge

@@ -602,6 +762,7 @@ function showEdgeDetail(edge) {
From: ${escapeHtml(sourceNode.label || source.id())}
To: ${escapeHtml(targetNode.label || target.id())}
+ ${edgeCostHtml}

Schema carried by this edge

${escapeHtml(edgeSchema || '(schema unavailable)')}

How the schema is produced

@@ -621,7 +782,17 @@ function renderScopeSummary(selected) { })); const scope = selected.length === 1 ? 'Single query' : `Batch workload · ${selected.length} queries`; const strategyText = strategies.size ? `Winning strategies: ${Array.from(strategies).join(', ')}` : 'No selected replacements'; - scopePickerEl.innerHTML = `
View scope
${escapeHtml(scope)}${escapeHtml(strategyText)}
`; + // Single query: use the exporter's own precomputed `NamedGraph.workload_cost` + // directly. Multiple queries: no single precomputed field covers exactly + // this subset, so aggregate the explicit per-decision annotations already + // in the export (dedup by `decision.id`) — see + // computeSelectionWorkloadCost's own doc for why this is aggregation, not + // client-side cost estimation. + const costSummary = selected.length === 1 ? selected[0].workload_cost : computeSelectionWorkloadCost(selected); + const costHtml = costSummary + ? `
Workload cost
${renderCostAnnotation('Baseline', costSummary.baseline_cost)}${renderCostAnnotation('Selected', costSummary.selected_cost)}${renderCostAnnotation('Benefit', costSummary.benefit)}
` + : ''; + scopePickerEl.innerHTML = `
View scope
${escapeHtml(scope)}${escapeHtml(strategyText)}
${costHtml}`; } function translationsForNode(query, node, stage) { @@ -661,6 +832,7 @@ function showPrePostDetail(data) {
${entry.role === 'replacement_root' ? 'This node replaces the pre-ASAP target.' : 'This node is generated or carried inside the replacement region.'}
${escapeHtml(entry.rationale || 'No rationale recorded.')}
${entry.target_pre_id === undefined ? `decision #${entry.id}` : `pre-ASAP target node #${entry.target_pre_id}`} · output ${escapeHtml(entry.output_kind || node.kind)}
+ ${renderDecisionCostBlock(entry)} `).join(''); translationHtml = `

Why this post-ASAP translation

${cards}
`; } else if (data.stage === 'post') { From 22d5d46cc5ef604e0f99b377f731732259c6b2e5 Mon Sep 17 00:00:00 2001 From: zz_y Date: Thu, 27 Aug 2026 09:32:34 -0600 Subject: [PATCH 2/2] fix(dag-viewer): address review findings on cost annotations (#286) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes 5 confirmed bugs from PR review plus 3 lower-priority follow-ups. Confirmed bugs: 1. viewer.js: `loadFiles()`/`loadWorkload()` dropped `workload_cost` from the object pushed onto `queries` — the single-query "Workload cost" panel (renderScopeSummary's `selected[0].workload_cost` read) was silently `undefined` on every interactive load path (drag-and-drop, file picker, and the planner/embedded path), even though the exported JSON carried the data. Both loaders now forward `workload_cost`. 2. viewer.js: `computeSelectionWorkloadCost` deduped decisions across a multi-query selection by bare `decision.id`, which is only unique within one `dag_export` process invocation, not across independently loaded files — a real collision (two files reusing the same small integer id) would silently drop one file's cost from the aggregate. Added a `sourceBatch` id assigned once per loaded document/file and changed the dedup key to `${sourceBatch}:${decision.id}`. 3. dag_export.rs: `shared_node_edge_annotations` counted edge occurrences (`Vec`) rather than distinct consuming nodes as `consumer_count` — a `Join` whose left and right operands are the same `Rc` (post pointer-dedup) inflated `consumer_count` to 2 for one real downstream consumer, halving the reported per-edge cost and producing two colliding `(from, to)` `EdgeCostAnnotation` entries (which `edgeCostByPair` in viewer.js then silently collapsed via Map overwrite). Switched to a `HashSet` per child so a single parent referencing the same shared child twice counts as one consumer. Added a regression test plus updated the existing edge-annotation test to use two genuinely distinct parents. 4. cost.rs: `total_cost()` validated `horizon` but not `recurring_cost_rate`/`one_shot_cost` themselves — a non-finite rate (e.g. a stray NaN from a future #287 caller) silently produced `Some(NaN)` instead of `None`, violating the module's own "never fabricate, never a poisoned total" rule. Both inputs are now validated finite before use; added regression tests. 5. dag_export.rs: `NamedGraph.workload_cost`'s doc claimed cross-query dedup via `workload_node_id`, but the actual producer (`decision_cost_entries`) only dedups within one query by `decision.id` — a reader trusting the doc and summing `NamedGraph.workload_cost` across queries would double-count a target shared between them. Corrected the doc to state the per-query-only scope and point cross-query readers at `WorkloadGraph.workload_cost` instead (the implementation was already correct; only the doc was wrong). Lower-priority follow-ups also addressed: - dag_export.rs: the legacy scalar `cost: f64` on `DagDecision`/ `TargetReplacement` is now derived from `selected_cost.value` at both call sites instead of being set independently from `winner.cost` a second time, closing the "kept in sync by convention only" gap the review flagged. - dag_export.rs: `default_cse_recompute_cost` is now memoized once per winner (`per_consumer_recompute_costs`, built right after `winners`) instead of being recomputed on every `winner_cost_annotations` call — a winner's target can be reached from more than one node position (internal sharing within a query, or the same CSE-shared target across several queries), so this avoided redundant subtree walks. - dag_export.rs (types crate): `shared_node_edge_annotations`'s `parents_of` map is now built inline inside `deduplicate_pointer_shared_nodes`'s existing per-node loop (which already visits every remapped child edge once while assigning final ids) instead of a second full pass over the deduplicated node list. Not fixed (noted only): `computeSelectionWorkloadCost` in viewer.js still hand-reimplements `cost.rs`'s `sum_workload_costs`/`workload_cost_summary` dedup-and-sum algorithm in JS, with no shared source of truth — there's no JS/Rust code-sharing mechanism in this tool today, so keeping the two algorithms in sync remains a manual/review responsibility. Flagged as a follow-up in the PR description. Testing: `cargo build --workspace`, `cargo test --workspace` (all green, no regressions), `cargo clippy --workspace --all-targets -- -D warnings` (clean), `cargo fmt --all -- --check` (clean, after running `cargo fmt --all` once for pre-existing drift), and `python3 -m unittest discover -s tools/dag-viewer` (18/18). Regenerated `dag.example.json` via generate-sample.sh — byte-identical, since the sample workload doesn't happen to exercise the same-parent-twice edge case fixed in item 3. `node --check` remains unavailable in this sandbox (no Node.js installed); verified the viewer.js changes by careful manual review plus the Python test suite, which inlines and structurally checks viewer.js. Co-Authored-By: Claude Sonnet 5 --- crates/devtools/src/bin/dag_export.rs | 90 ++++++++++++---- crates/types/src/cost.rs | 65 +++++++++-- crates/types/src/dag_export.rs | 148 +++++++++++++++++++++----- tools/dag-viewer/viewer.js | 51 ++++++--- 4 files changed, 285 insertions(+), 69 deletions(-) diff --git a/crates/devtools/src/bin/dag_export.rs b/crates/devtools/src/bin/dag_export.rs index e5f9ab4..8eb21cf 100644 --- a/crates/devtools/src/bin/dag_export.rs +++ b/crates/devtools/src/bin/dag_export.rs @@ -79,15 +79,23 @@ const STRUCTURAL_COST_MODEL_VERSION: &str = "dag_export-structural-cost-v1"; /// carried. /// /// The baseline is always [`BaselineRef::PreAsapRecomputation`]: the cost of -/// recomputing `target` independently at every one of its `consumer_count` -/// use sites, via -/// [`default_cse_recompute_cost`] — the exact structural-size proxy -/// `asap_aware_mapping::cost_model`'s own `DefaultCostModel` already uses, -/// not a second formula. This baseline is always computable (never -/// `Unavailable`) since it only needs `target` itself, unlike `selected`, -/// which is `Unavailable` whenever `selected_cost` is `NaN` (the plugged-in -/// cost model has no numeric estimate for that particular candidate shape — -/// see [`RankedGroup::costs`](asap_aware_mapping::replacement::RankedGroup::costs)'s +/// recomputing the winner's own target independently at every one of its +/// `consumer_count` use sites. `per_consumer_recompute` is that one-site cost +/// — [`default_cse_recompute_cost`] applied to the target, the exact +/// structural-size proxy `asap_aware_mapping::cost_model`'s own +/// `DefaultCostModel` already uses, not a second formula — passed in +/// pre-computed rather than taking `target: &QueryExpr` and recomputing it +/// here: this function is called once per matched *node position* a winner +/// produced or carried (once per `target_replacement` flat entry, once per +/// `find_winner` hit), and the same winner's target can be reached from more +/// than one node position, so callers memoize `default_cse_recompute_cost` +/// once per winner (see `run_post_asap_with_progress`'s +/// `per_consumer_recompute_costs`) instead of this function re-walking the +/// identical subtree on every call. This baseline is always computable +/// (never `Unavailable`), unlike `selected`, which is `Unavailable` whenever +/// `selected_cost` is `NaN` (the plugged-in cost model has no numeric +/// estimate for that particular candidate shape — see +/// [`RankedGroup::costs`](asap_aware_mapping::replacement::RankedGroup::costs)'s /// own doc). /// /// Every value here is unit-tagged [`CostUnit::RelativeStructuralUnits`], @@ -96,11 +104,10 @@ const STRUCTURAL_COST_MODEL_VERSION: &str = "dag_export-structural-cost-v1"; /// (issue #287's job) — see `asap_types::cost`'s module doc for why this /// crate refuses to mislabel a structural proxy as a real rate. fn winner_cost_annotations( - target: &QueryExpr, + per_consumer_recompute: f64, consumer_count: usize, selected_cost: f64, ) -> (CostAnnotation, CostAnnotation, CostAnnotation) { - let per_consumer_recompute = default_cse_recompute_cost(target).0; let baseline_value = per_consumer_recompute * consumer_count.max(1) as f64; let baseline = CostAnnotation::modeled( baseline_value, @@ -414,7 +421,8 @@ fn decision_cost_entries(graph: &DagGraph) -> Vec<(u32, CostAnnotation, CostAnno if !seen.insert(decision.id) { continue; } - let (Some(baseline), Some(selected)) = (&decision.baseline_cost, &decision.selected_cost) else { + let (Some(baseline), Some(selected)) = (&decision.baseline_cost, &decision.selected_cost) + else { continue; }; entries.push((decision.id, baseline.clone(), selected.clone())); @@ -423,11 +431,15 @@ fn decision_cost_entries(graph: &DagGraph) -> Vec<(u32, CostAnnotation, CostAnno } /// Build a [`TargetReplacement`] for `winner`, matching this file's own -/// per-target `before`/`after` construction. +/// per-target `before`/`after` construction. `per_consumer_recompute` is +/// `winner`'s own memoized [`default_cse_recompute_cost`] — see +/// `winner_cost_annotations`'s own doc for why callers pass this in instead +/// of recomputing it here. fn target_replacement( decision_id: u32, target_pre_id: u32, winner: &Winner<'_>, + per_consumer_recompute: f64, ) -> TargetReplacement { let strategy = winner.candidate.strategy.to_string(); let before = dag_export::export(winner.target); @@ -440,14 +452,20 @@ fn target_replacement( } }; let (baseline_cost, selected_cost, benefit) = - winner_cost_annotations(winner.target, winner.consumer_count, winner.cost); + winner_cost_annotations(per_consumer_recompute, winner.consumer_count, winner.cost); + // Derived from `selected_cost` (not read from `winner.cost` a second + // time) so the legacy scalar field and the structured annotation can + // never drift apart at this call site — see `winner_cost_annotations`'s + // own doc for why `selected_cost.value` is `None` (and this is `NaN`) + // in exactly the same case `winner.cost` itself would already be `NaN`. + let cost = selected_cost.value.unwrap_or(f64::NAN); TargetReplacement { decision_id, target_pre_id, strategy, rationale: decision_rationale(winner), rank: 0, - cost: winner.cost, + cost, before, after, baseline_cost: Some(baseline_cost), @@ -577,6 +595,20 @@ fn run_post_asap_with_progress( }) .collect(); + // One `default_cse_recompute_cost` walk per *winner*, not per node a + // winner's decision ends up cloned onto: `winner_cost_annotations` is + // called once per matched node position — once per `target_replacement` + // flat entry, and once per `find_winner` hit inside `export_post_asap`'s + // traversal — and a single target can be reached from more than one + // node position (an internally-shared subtree within one query, or the + // same CSE-shared target reached from several queries), so recomputing + // this per call would re-walk the identical subtree redundantly. + // Indexed in parallel with `winners`. + let per_consumer_recompute_costs: Vec = winners + .iter() + .map(|winner| default_cse_recompute_cost(winner.target).0) + .collect(); + // `by_hash` only narrows the search; `lookup_winner`'s own structural // equality check is the real decision — see that function's doc. let mut by_hash_cache = HashCache::new(); @@ -615,14 +647,20 @@ fn run_post_asap_with_progress( let mut find_winner = |expr: &QueryExpr| -> Option { let i = lookup_winner(&by_hash, &winners, &mut post_graph_cache, expr)?; let winner = &winners[i]; - let (baseline_cost, selected_cost, benefit) = - winner_cost_annotations(winner.target, winner.consumer_count, winner.cost); + let (baseline_cost, selected_cost, benefit) = winner_cost_annotations( + per_consumer_recompute_costs[i], + winner.consumer_count, + winner.cost, + ); + // Derived from `selected_cost`, not `winner.cost` a second time — + // see `target_replacement`'s identical derivation for why. + let cost = selected_cost.value.unwrap_or(f64::NAN); let decision = DagDecision { id: i as u32, strategy: winner.candidate.strategy.to_string(), rationale: decision_rationale(winner), rank: 0, - cost: winner.cost, + cost, role: "replacement_region", baseline_cost: Some(baseline_cost), selected_cost: Some(selected_cost), @@ -672,7 +710,12 @@ fn run_post_asap_with_progress( if let Some(i) = lookup_winner(&by_hash, &winners, &mut lookup_cache, source_expr) { replacements.push(( name.clone(), - target_replacement(i as u32, node.id, &winners[i]), + target_replacement( + i as u32, + node.id, + &winners[i], + per_consumer_recompute_costs[i], + ), )); matched[i] = true; } @@ -855,11 +898,16 @@ async fn main() { continue; } match asap_types::cost::workload_cost_summary( - entries.iter().map(|(id, baseline, selected)| (Some(*id), baseline, selected)), + entries + .iter() + .map(|(id, baseline, selected)| (Some(*id), baseline, selected)), "dag_export-workload-cost-v1", ) { Ok(summary) => query.workload_cost = Some(summary), - Err(mismatch) => eprintln!("dag_export: workload cost aggregation for {:?} skipped — {mismatch}", query.name), + Err(mismatch) => eprintln!( + "dag_export: workload cost aggregation for {:?} skipped — {mismatch}", + query.name + ), } workload_entries.extend(entries); } diff --git a/crates/types/src/cost.rs b/crates/types/src/cost.rs index 0ddc84e..924f7d9 100644 --- a/crates/types/src/cost.rs +++ b/crates/types/src/cost.rs @@ -236,11 +236,32 @@ pub fn benefit_ratio(baseline_value: f64, delta: f64) -> Option { /// `total_cost(H) = recurring_cost_rate * H + one_shot_cost` — finite-run or /// one-shot totals require an explicit horizon. Returns `None` (never a -/// fabricated total) when both terms are `None`, or when `horizon` isn't a -/// finite, non-negative number for a `Some(rate)`. Rate and one-shot costs -/// are passed as separate arguments specifically so they can never be -/// silently added by a caller before this function ever sees them. -pub fn total_cost(recurring_cost_rate: Option, horizon: f64, one_shot_cost: Option) -> Option { +/// fabricated or poisoned total) when both terms are `None`, when `horizon` +/// isn't a finite, non-negative number for a `Some(rate)`, or when either +/// `recurring_cost_rate` or `one_shot_cost` is itself non-finite (`NaN` or +/// infinite) — a non-finite input must never silently produce `Some(NaN)`. +/// Rate and one-shot costs are passed as separate arguments specifically so +/// they can never be silently added by a caller before this function ever +/// sees them. +pub fn total_cost( + recurring_cost_rate: Option, + horizon: f64, + one_shot_cost: Option, +) -> Option { + // A non-finite input anywhere here (NaN/±inf — e.g. from a future #287 + // caller upstream) must never quietly poison the total into `Some(NaN)`: + // that would violate this module's own "never fabricate, never a + // poisoned total" rule as much as inventing a number from nothing would. + if let Some(rate) = recurring_cost_rate { + if !rate.is_finite() { + return None; + } + } + if let Some(one_shot) = one_shot_cost { + if !one_shot.is_finite() { + return None; + } + } let recurring = match recurring_cost_rate { Some(rate) => { if !horizon.is_finite() || horizon < 0.0 { @@ -371,11 +392,19 @@ pub fn workload_cost_summary<'a, I>( where I: IntoIterator, &'a CostAnnotation, &'a CostAnnotation)> + Clone, { - let baseline_cost = sum_workload_costs(entries.clone().into_iter().map(|(id, baseline, _)| (id, baseline)))?; - let selected_cost = sum_workload_costs(entries.into_iter().map(|(id, _, selected)| (id, selected)))?; + let baseline_cost = sum_workload_costs( + entries + .clone() + .into_iter() + .map(|(id, baseline, _)| (id, baseline)), + )?; + let selected_cost = + sum_workload_costs(entries.into_iter().map(|(id, _, selected)| (id, selected)))?; let benefit = match (baseline_cost.value, selected_cost.value) { - (Some(baseline_value), Some(selected_value)) if baseline_cost.unit == selected_cost.unit => { + (Some(baseline_value), Some(selected_value)) + if baseline_cost.unit == selected_cost.unit => + { let delta = baseline_value - selected_value; CostAnnotation { value: Some(delta), @@ -456,6 +485,26 @@ mod tests { assert_eq!(total_cost(Some(2.0), -1.0, None), None); } + /// A non-finite `recurring_cost_rate` (e.g. a stray `NaN` from a future + /// #287 caller) must never silently produce `Some(NaN)` — that's a + /// poisoned total, exactly the kind of fabricated-looking value this + /// module's "never fabricate" rule exists to prevent. + #[test] + fn total_cost_rejects_a_non_finite_recurring_rate() { + assert_eq!(total_cost(Some(f64::NAN), 5.0, None), None); + assert_eq!(total_cost(Some(f64::INFINITY), 5.0, None), None); + assert_eq!(total_cost(Some(f64::NEG_INFINITY), 5.0, Some(1.0)), None); + } + + /// Same guard on the one-shot addend — a `NaN`/infinite one-shot cost + /// must not poison the total either, even when the recurring side is + /// perfectly well-formed. + #[test] + fn total_cost_rejects_a_non_finite_one_shot_cost() { + assert_eq!(total_cost(Some(2.0), 5.0, Some(f64::NAN)), None); + assert_eq!(total_cost(None, 5.0, Some(f64::INFINITY)), None); + } + fn ann(value: f64, unit: CostUnit) -> CostAnnotation { CostAnnotation::modeled(value, unit, "v1", vec![]) } diff --git a/crates/types/src/dag_export.rs b/crates/types/src/dag_export.rs index 4048699..74d8722 100644 --- a/crates/types/src/dag_export.rs +++ b/crates/types/src/dag_export.rs @@ -46,8 +46,8 @@ use std::rc::Rc; use serde::Serialize; -use crate::post_asap::{SummaryExpr, SummaryNode}; use crate::cost::{CostAnnotation, CostInput, CostUnit}; +use crate::post_asap::{SummaryExpr, SummaryNode}; use crate::pre_asap::cse::{dag_node_count, structural_hash, HashCache}; use crate::pre_asap::query_expr::{QueryExpr, Source}; @@ -242,15 +242,22 @@ pub struct NamedGraph { /// `NamedGraph` is unaffected. #[serde(default, skip_serializing_if = "Option::is_none")] pub post_graph: Option, - /// This query's whole selected-workload cost/benefit — one of issue - /// #286's granularity items. Built by summing this query's own + /// This query's own selected-workload cost/benefit — one of issue + /// #286's granularity items. Built by summing *this query's own* /// `post_graph` decision-node cost annotations, deduplicated by - /// `workload_node_id` (a node this query shares with an earlier query - /// in the same export is still counted once here, since - /// `assign_workload_node_ids` assigns identity workload-wide, not - /// per-query). `None` unless a higher layer built one (same + /// `decision.id` **within this one query only** (a decision spanning + /// several nodes in this query's own replacement region is still + /// counted once here). `None` unless a higher layer built one (same /// `--post-asap`-gated pattern as `post_graph`); omitted from JSON when /// absent. + /// + /// This does **not** dedupe across queries: a target shared by two + /// queries (e.g. a common `Scan` after workload-wide CSE) is counted + /// once in *each* query's own `workload_cost` — summing several + /// `NamedGraph.workload_cost` values by hand double-counts any decision + /// shared between them. For a cross-query total that dedupes correctly, + /// use [`WorkloadGraph::workload_cost`] instead, which is built + /// specifically to cover every query in one pass. #[serde(default, skip_serializing_if = "Option::is_none")] pub workload_cost: Option, } @@ -680,6 +687,18 @@ fn deduplicate_pointer_shared_nodes(nodes: Vec, root: u32) -> DagGraph let mut by_source_ptr = HashMap::::new(); let mut old_to_new = vec![0_u32; nodes.len()]; let mut deduplicated = Vec::with_capacity(nodes.len()); + // Built in the same pass as `deduplicated` itself, rather than by a + // second full walk over `deduplicated` afterward (see + // `shared_node_edge_annotations`'s own doc for why one pass suffices): + // `HashSet`, not `Vec`, per child — a *distinct* consuming node is what + // "consumer" means here. A single parent can reference the same shared + // child from two of its own operand slots at once (e.g. a `Join` whose + // left and right are the same `Rc` post pointer-dedup) — that's one + // downstream consumer reading the child twice, structurally, not two + // separate consumers, and it must not inflate `consumer_count` (which + // would understate `per_edge_cost`) or produce two colliding `(from, + // to)` `EdgeCostAnnotation` entries for the exact same edge. + let mut parents_of: HashMap> = HashMap::new(); for mut node in nodes { node.children = node @@ -701,10 +720,21 @@ fn deduplicate_pointer_shared_nodes(nodes: Vec, root: u32) -> DagGraph by_source_ptr.insert(source_ptr, new_id); } old_to_new[old_id as usize] = new_id; + // `node.children` is already remapped to final new-id space above, + // and post-order guarantees every child was already pushed (with + // its own final id) before this parent is reached — so this is safe + // to record right here, once, rather than re-deriving it from + // `deduplicated` in a later pass. A node this loop *doesn't* push + // (the dedup-hit `continue` above) never reaches this line, but that + // never loses information: its first-pushed duplicate already had + // its own (identical) children recorded when *it* was processed. + for &child_new_id in &node.children { + parents_of.entry(child_new_id).or_default().insert(new_id); + } deduplicated.push(node); } - let edge_annotations = shared_node_edge_annotations(&deduplicated); + let edge_annotations = shared_node_edge_annotations(&deduplicated, &parents_of); DagGraph { nodes: deduplicated, root: old_to_new[root as usize], @@ -724,21 +754,24 @@ fn deduplicate_pointer_shared_nodes(nodes: Vec, root: u32) -> DagGraph /// evenly divided across every consuming edge — never a guessed multi-hop /// path cost (explicitly out of scope per the issue). /// +/// `parents_of` is [`deduplicate_pointer_shared_nodes`]'s own +/// already-built child -> distinct-parents map, passed in rather than +/// rebuilt here by a second walk over `nodes`'s edges — that caller has +/// already visited every edge exactly once while assigning final node ids, +/// so redoing the same walk here would just re-derive what it already +/// knows. +/// /// Uses [`dag_node_count`] (the same structural-size proxy /// `asap_aware_mapping::cost_model::default_cse_recompute_cost` computes; /// duplicated here in plain terms since `asap_types` may not depend on that /// higher crate) rather than a real per-byte transfer cost — honestly /// unit-tagged [`CostUnit::RelativeStructuralUnits`], not a rate. -fn shared_node_edge_annotations(nodes: &[DagNode]) -> Vec { - let mut parents_of: HashMap> = HashMap::new(); - for node in nodes { - for &child in &node.children { - parents_of.entry(child).or_default().push(node.id); - } - } - +fn shared_node_edge_annotations( + nodes: &[DagNode], + parents_of: &HashMap>, +) -> Vec { let mut annotations = Vec::new(); - for (child_id, parents) in &parents_of { + for (child_id, parents) in parents_of { if parents.len() < 2 { continue; } @@ -1414,16 +1447,23 @@ mod tests { #[test] fn export_post_asap_annotates_edges_into_a_genuinely_shared_node() { - // Two parents (a Join's own two sides) share the exact same `Rc` - // Scan — `export_post_asap`'s `deduplicate_pointer_shared_nodes` - // must merge them onto one node id, and (issue #286) attach an + // Two *distinct* parents (a Dedup and a Limit, each with their own + // single child slot) share the exact same `Rc` Scan — + // `export_post_asap`'s `deduplicate_pointer_shared_nodes` must merge + // them onto one node id, and (issue #286) attach an // `EdgeCostAnnotation` on each of the two edges running into it. let shared_scan = Rc::new(scan("metrics", value_col())); - let root = QueryExpr::Join { - kind: crate::pre_asap::query_expr::JoinKind::Inner, - pred: Predicate(Rc::new(QueryExpr::Literal(ScalarValue::Boolean(true)))), - left: Rc::clone(&shared_scan), - right: Rc::clone(&shared_scan), + let left_branch = QueryExpr::Dedup { + cols: vec![0], + child: Rc::clone(&shared_scan), + }; + let right_branch = QueryExpr::Limit { + n: 5, + offset: 0, + child: Rc::clone(&shared_scan), + }; + let root = QueryExpr::Concat { + children: vec![left_branch, right_branch], }; let graph = export_post_asap(&root, &mut |_| None); @@ -1438,14 +1478,66 @@ mod tests { .iter() .filter(|edge| edge.from == scan_id) .collect(); - assert_eq!(edges_into_scan.len(), 2, "one annotation per consuming edge"); + assert_eq!( + edges_into_scan.len(), + 2, + "one annotation per distinct consuming parent" + ); + let distinct_parents: std::collections::HashSet<_> = + edges_into_scan.iter().map(|edge| edge.to).collect(); + assert_eq!( + distinct_parents.len(), + 2, + "the two parents (Dedup, Limit) must be distinct" + ); for edge in &edges_into_scan { - assert_eq!(edge.cost.value, Some(0.5), "1 unique node / 2 consumers"); - assert_eq!(edge.cost.unit, crate::cost::CostUnit::RelativeStructuralUnits); + assert_eq!( + edge.cost.value, + Some(0.5), + "1 unique node / 2 distinct consumers" + ); + assert_eq!( + edge.cost.unit, + crate::cost::CostUnit::RelativeStructuralUnits + ); assert_eq!(edge.cost.source, crate::cost::CostSource::Modeled); } } + /// Regression test: a single parent referencing the same shared child + /// from two of its own operand slots at once (a `Join` whose left and + /// right sides are the exact same `Rc`, post pointer-dedup) is *one* + /// downstream consumer, not two — this must not inflate + /// `consumer_count`, must not halve the reported per-edge cost, and + /// must not produce two colliding `(from, to)` `EdgeCostAnnotation` + /// entries for what is structurally a single edge. Since there is only + /// one *distinct* consumer here, this shape isn't "genuinely shared" at + /// all in the `>= 2 distinct consumers` sense `shared_node_edge_annotations` + /// requires, so no annotation should be produced for it. + #[test] + fn a_single_parent_referencing_a_shared_child_twice_is_one_consumer_not_two() { + let shared_scan = Rc::new(scan("metrics", value_col())); + let root = QueryExpr::Join { + kind: crate::pre_asap::query_expr::JoinKind::Inner, + pred: Predicate(Rc::new(QueryExpr::Literal(ScalarValue::Boolean(true)))), + left: Rc::clone(&shared_scan), + right: Rc::clone(&shared_scan), + }; + let graph = export_post_asap(&root, &mut |_| None); + + assert_eq!( + graph.nodes.iter().filter(|n| n.kind == "Scan").count(), + 1, + "the shared Scan must be merged onto one node, not duplicated" + ); + assert!( + graph.edge_annotations.is_empty(), + "a single parent referencing the same child twice is one consumer, not a genuine \ + multi-consumer share — got: {:?}", + graph.edge_annotations + ); + } + #[test] fn export_never_produces_edge_annotations_since_it_never_shares_nodes() { // Plain `export` (no `export_post_asap`) never deduplicates by `Rc` diff --git a/tools/dag-viewer/viewer.js b/tools/dag-viewer/viewer.js index 509acd9..c227f21 100644 --- a/tools/dag-viewer/viewer.js +++ b/tools/dag-viewer/viewer.js @@ -20,7 +20,12 @@ cytoscape.use(window.cytoscapeDagre); // like "SummaryAgg" mixed in, and any such node has no `hash` — there's no // corresponding QueryExpr to hash) — left `undefined` when absent (omitted // whenever --post-asap wasn't set, or this query had zero replacements), -// unlike `replacements` which always defaults to an array. +// unlike `replacements` which always defaults to an array. `workload_cost` +// is the optional per-query `NamedGraph.workload_cost` (issue #286), also +// left `undefined` when absent. `sourceBatch` is a viewer-assigned integer +// (never present in the JSON itself) shared by every query loaded from the +// same document — see computeSelectionWorkloadCost's own doc for why it +// exists and how it's used. let queries = []; let activeIndex = -1; let cy = null; @@ -29,6 +34,13 @@ let zoom = 1; // The viewer has one Pre/Post-ASAP mode. One selected query renders its own // two DAGs; multiple selected queries union each stage into one workload DAG. let participants = new Set(); +// Every query pushed from the *same* loaded JSON document (one `dag_export` +// process invocation) shares one `sourceBatch` id, assigned here. Needed +// because `DagDecision.id` is only unique *within* one dag_export run, not +// across independently-generated files — computeSelectionWorkloadCost below +// dedups by `${sourceBatch}:${decision.id}`, never `decision.id` alone, so +// two files that happen to reuse the same small integer id never collide. +let nextSourceBatch = 0; const dropzone = document.getElementById('dropzone'); const fileInput = document.getElementById('fileInput'); @@ -95,11 +107,14 @@ function loadFiles(fileList) { const parsed = JSON.parse(reader.result); const incoming = parsed.queries || []; const existingNames = new Set(queries.map((q) => q.name)); + // One batch id per *file* — every query this one dag_export + // invocation produced shares its decision.id numbering. + const sourceBatch = nextSourceBatch++; incoming.forEach((q) => { let name = q.name; if (existingNames.has(name)) name = `${q.name} (${file.name})`; existingNames.add(name); - queries.push({ name, graph: q.graph, source: q.source, replacements: q.replacements || [], post_graph: q.post_graph }); + queries.push({ name, graph: q.graph, source: q.source, replacements: q.replacements || [], post_graph: q.post_graph, workload_cost: q.workload_cost, sourceBatch }); }); } catch (err) { alert(`Failed to parse ${file.name}: ${err.message}`); @@ -658,13 +673,21 @@ function nodeCostBadgeSuffix(node) { } // Workload-wide baseline/selected/benefit for the currently selected -// queries, deduplicated by `decision.id` — the same collision-free key -// `crates/devtools/src/bin/dag_export.rs`'s own `decision_cost_entries` -// dedupes by (a decision spans every node in its replacement region, and a -// CSE-shared target can appear in more than one selected query). This -// aggregates explicit per-node `CostAnnotation`s already in the export; it -// never estimates a cost itself. Returns `null` when nothing in the -// selection carries a cost annotation, or when selected annotations +// queries, deduplicated by `decision.id` *within one loaded document* — the +// same collision-free key `crates/devtools/src/bin/dag_export.rs`'s own +// `decision_cost_entries` dedupes by (a decision spans every node in its +// replacement region, and a CSE-shared target can appear in more than one +// selected query from the same dag_export run). `decision.id` is only +// unique within the one `dag_export` process invocation that produced it, +// never across independently-generated files — the viewer explicitly +// supports loading and selecting across several files at once — so the +// dedup key is `${query.sourceBatch}:${decision.id}`, not `decision.id` +// alone; two files that happen to reuse the same small integer id must +// never collide and silently drop one file's cost from the total. +// +// This aggregates explicit per-node `CostAnnotation`s already in the +// export; it never estimates a cost itself. Returns `null` when nothing in +// the selection carries a cost annotation, or when selected annotations // disagree on unit (unit-incompatible aggregation is refused, not mixed). function computeSelectionWorkloadCost(selected) { const seenDecisions = new Set(); @@ -676,8 +699,10 @@ function computeSelectionWorkloadCost(selected) { const nodes = (query.post_graph && query.post_graph.nodes) || []; for (const node of nodes) { const decision = node.decision; - if (!decision || seenDecisions.has(decision.id)) continue; - seenDecisions.add(decision.id); + if (!decision) continue; + const dedupKey = `${query.sourceBatch}:${decision.id}`; + if (seenDecisions.has(dedupKey)) continue; + seenDecisions.add(dedupKey); const baseline = decision.baseline_cost; const selectedCost = decision.selected_cost; if (!baseline || !selectedCost) continue; @@ -968,7 +993,9 @@ document.getElementById('resetBtn').addEventListener('click', () => { zoom = 1; function loadWorkload(parsed) { const incoming = (parsed && parsed.queries) || []; - incoming.forEach((q) => queries.push({ name: q.name, graph: q.graph, source: q.source, replacements: q.replacements || [], post_graph: q.post_graph })); + // One batch id for this whole document — see `sourceBatch`'s own doc above. + const sourceBatch = nextSourceBatch++; + incoming.forEach((q) => queries.push({ name: q.name, graph: q.graph, source: q.source, replacements: q.replacements || [], post_graph: q.post_graph, workload_cost: q.workload_cost, sourceBatch })); if (activeIndex === -1 && queries.length > 0) activeIndex = 0; if (participants.size === 0 && activeIndex >= 0) participants.add(activeIndex); }