feat(dag-viewer): show planner cost and benefit annotations - #296
Open
zzylol wants to merge 4 commits into
Open
Conversation
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<EdgeCostAnnotation>`, 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<WorkloadCostSummary>`. - 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
zzylol
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #286.
Summary
Adds structured cost and benefit annotations to DAG exports and renders them in the DAG viewer.
What changed
CostAnnotation, cost units, provenance, baselines, benefit deltas and ratios.Current estimates use
RelativeStructuralUnits. Recurrence-aware cost rates and measured benchmark costs remain deferred to #287 and #288.Correctness fixes included
workload_costthrough every viewer loading path.total_cost.Merge resolution
Merged the latest
mainand retained both sets of additive DAG metadata: this PRs cost annotations and the accuracy guarantees/rejections added onmain.Validation
cargo check -p asap-types -p asap-devtools --bin dag_exportcargo test -p asap-types— 130 passedcargo test -p asap-devtools --bin dag_export— 6 passedpython3 -m unittest discover -s tools/dag-viewer -p "test_*.py"— 18 passedcargo fmt --all -- --checkgit diff --checkNode.js is not installed in the review environment, so
node --checkwas unavailable.