feat: rocky preview diff + cost — Phase 2/3 substance#280
Merged
hugocorreia90 merged 3 commits intomainfrom Apr 29, 2026
Merged
feat: rocky preview diff + cost — Phase 2/3 substance#280hugocorreia90 merged 3 commits intomainfrom
hugocorreia90 merged 3 commits intomainfrom
Conversation
Lifts the rocky preview diff/cost commands from wire-contract stubs to working substance. Pure RunRecord-pair builders that the Phase 4 GitHub Action will compose into a single PR comment. **preview diff (Phase 2 substance):** - build_preview_diff pairs branch and base RunRecords by model name and reports rows_added / rows_removed via signed delta of rows_affected. - Each per-model entry carries sampling_window with coverage = "not_yet_sampled" + coverage_warning = true — honest hedge that row content changes that don't move row counts won't surface here. Phase 2.5 lifts to checksum-bisection exhaustive sampling (datafold-style). - render_preview_diff_markdown: deterministic GitHub-flavored Markdown. **preview cost (Phase 3 substance):** - build_preview_cost_delta pairs branch and base over the union of model names. Three cases: paired (full delta), branch-only (delta == branch cost — new in PR), base-only (skipped_via_copy = true; base cost rolls into savings_from_copy_usd). - Reuses compute_observed_cost_usd from rocky_core::cost — same formula rocky cost latest uses, so the surfaces stay consistent. - Best-effort cost-param resolution: missing config → durations + bytes ship without USD. Branch-vs-base run pairing: scans the most recent 50 RunRecords from state store, takes the newest RunRecord with git_branch == branch_name as branch, newest with a different git_branch as base. Tighter partitioning lands when state-store branch records carry a paired base-run pointer. 10 new unit tests covering: signed row-count deltas, copy-savings roll-up, branch-only model semantics, missing-config path, DuckDB zero-cost path, Markdown rendering for both surfaces. Total preview unit tests: 24 (was 14).
Two leftover doc-comment references to a local design-doc path (`~/Developer/...`) that aren't accessible to anyone reading the code on GitHub. Replaced the rocky-cli/src/output.rs reference with a self-contained explanation; removed the rocky-cli/src/commands/ preview.rs reference entirely (the module-level doc already names the three subcommands).
Re-runs codegen to propagate the path-stripped DiscoverOutput schemas_cached doc comment from output.rs into the JSON schema + Pydantic + TypeScript bindings. No behavior change.
This was referenced Apr 29, 2026
hugocorreia90
added a commit
that referenced
this pull request
Apr 29, 2026
Engine 1.18.0 ships the rocky preview workflow end-to-end (#279, #280, #281, #282), the [budget].max_bytes_scanned threshold (#288), the audit-sweep closeout (#283, #285–#287, #290–#293), and the rocky-server auth + CORS gate (#291). Dagster 1.15.0 picks up the regenerated Pydantic models for the rocky preview surface and ships the P1 cluster (#289) + FR-014 follow-on (#284). VS Code 1.10.0 regenerates TypeScript bindings for rocky preview and RunCostSummary.total_bytes_scanned. See per-artifact CHANGELOG entries for the full breakdown.
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
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.
Summary
Lifts
rocky preview diffandrocky preview costfrom wire-contract stubs (PR #279) to working substance. PureRunRecord-pair builders that the future Phase 4 GitHub Action composes into a single PR comment.What's in this PR
Phase 2 substance —
rocky preview diff:build_preview_diff(branch, base)— pure pairing of twoRunRecords by model name. Computesrows_added/rows_removedvia signed delta ofrows_affected.sampling_windowwithcoverage = "not_yet_sampled"andcoverage_warning = true— honest hedge that row-content changes that don't move row counts won't surface here. Phase 2.5 lifts to checksum-bisection exhaustive sampling (datafold-style) over therocky_core::compareshadow kernel.render_preview_diff_markdown— deterministic GitHub-flavored Markdown table; the empty-runs path emits a setup hint, not silence.Phase 3 substance —
rocky preview cost:build_preview_cost_delta(branch, base, params)— pure pairing over the union of model names with three cases:delta_usd == branch_cost_usd(base is zero).skipped_via_copy = true,branch_*zero/None,base_cost_usdrolls intosummary.savings_from_copy_usd(the cost the PR avoided by copying).rocky_core::cost::compute_observed_cost_usd— the same formularocky cost latestuses — so the two surfaces stay consistent.delta_usdbecomesNonerather than wrong.render_preview_cost_markdown— deterministic Markdown with the headline Δ, branch/base totals, copy-savings, and per-model rows.Branch-vs-base pairing. Scans the most recent 50
RunRecords from the state store; newest withgit_branch == branch_nameis the branch run, newest with a differentgit_branchis the base run. Tighter partitioning (state-store branch records carrying a paired base-run pointer) is a follow-up.Cleanup. Stripped two leftover doc-comment references to a private design-doc path from
crates/rocky-cli/src/{output.rs,commands/preview.rs}— those paths aren't accessible to anyone reading the code on GitHub.What's NOT in this PR (deliberate)
rocky_core::compare's shadow kernel; needs warehouseQueryResultplumbing theRunRecorddoesn't carry today. The wire field is already populated honestly withcoverage_warning = true.Test plan
preview::tests):diff_row_delta_signs_correctly— branch > base → +rows; branch < base → −rows.diff_no_change_still_warns_about_coverage— coverage_warning fires even on identical runs.cost_delta_databricks_paired_models— duration-driven delta on paired models.cost_delta_skipped_via_copy_rolls_into_savings— base-only model rolls intosavings_from_copy_usd, nottotal_base_cost_usd.cost_delta_branch_only_model_delta_eq_branch_cost— new-in-PR model: delta == branch cost.cost_delta_no_params_emits_durations_only— missing config → cost None, durations populated.cost_delta_duckdb_always_zero— DuckDB delta is 0.0, not None.diff_markdown_renders_table— Markdown contains headline metrics + per-model rows + Phase 2.5 hedge.diff_markdown_empty_path_explains_setup— no-paired-runs path emits setup hint.cost_markdown_renders_table— Markdown headline + per-model + copy-skip count.cargo test -p rocky-cli --lib— 294 tests pass.cargo clippy -p rocky-cli --all-targets -- -D warningsclean.