Conversation
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.
Why
Closes #427. Planner APIs reused “implementation,” “bind,” and “boundary” for unrelated concepts, making candidate construction, schema resolution, window coverage, and physical transfers hard to distinguish.
What
Rename the concepts and their callers consistently:
Realization,RealizationError, andSummaryRealizationidentify candidate physical forms; internal enumeration becomesrealizations_for_intent.SchemaResolver::resolve_schemaandresolve_schema_with_inheritedreplace the schema binder family.PaneLayoutandWindowEdgeCoverage::ExactWindowEdgeResidualdescribe window coverage.physical_handoff_cost,PhysicalHandoffKind,PhysicalHandoffBytes, and the related evidence/profile/estimate types describe transfers and materialization.CandidatePlanningInputsandnew_with_planning_inputs*constructors name injected cost, accuracy, allocation, and evidence dependencies.Update documentation to use “comparison scope” and “planner-runtime contract,” including document paths and incoming links. Ordinary fluent builders and SQL/PromQL syntax retain their names.
How
This is a source API refactor with no new planning abstraction or algorithm. All workspace consumers use the new vocabulary. Explicit Serde names preserve deployed JSON keys and variants; model-version strings and exported resource labels remain stable.
Before this PR
A caller used
Binder::new().bind(&tree), cost-model extensions returnedImplementation, and a physical transfer usedBoundaryKindeven though pane coverage also used “boundary.”After this PR
The same caller uses
SchemaResolver::new().resolve_schema(&tree), returnsRealization, and declares aPhysicalHandoffKind. Window edges useWindowEdgeCoverage. The same queries and physical evidence produce the same plans and resource arithmetic.See the migration guide for the complete source-name mapping.
Evidence
boundariesandper_boundaryJSON keys and the expected 240-byte network estimate.boundarieskeys before parsing and selecting a physical plan.Verification
cargo test --workspace --locked: passed.cargo clippy --workspace --all-targets --all-features --locked -- -D warnings: passed.cargo fmt --all --checkandgit diff --check: passed.bash tools/verify_metricsql_external_consumer.sh: passed on stable Rust.python3 tools/verify_metricsql_vendored_baseline.py: passed; preserved the 21 library and 3 documentation upstream failures recorded by that baseline checker.asap-typesandasap-aware-mapping: built successfully; existing documentation warnings remain. Relative links in changed Markdown documents resolve.Architectural decisions
Use one Rust naming family per concept rather than retain old aliases alongside it. Migration notes cover the source break. Preserve wire names through Serde so downstream evidence files and exported contracts require no JSON migration.
Limitations and follow-up
This breaks Rust source imports and call sites using the old names; downstream crates must apply the migration table. Historical archived documentation is unchanged. This is implementation and verification by the same agent, not an independent review.
Human review — do not complete with an agent