refactor(backend): drop row_num/col_num legacy fallback from cms_params - #269
Merged
Merged
Conversation
Per-user direction: now that the asapcollector configs migrated to canonical `w` / `d` keys (sibling PR `sync-config-canonical-w-d`), the `row_num` / `col_num` fallback PR #268 added has no remaining caller. Drop it. Three sweeps: - `accumulator_factory::cms_params` — canonical keys only; `cms_params_reads_canonical_w_d_keys` test updated to drop the legacy half. - `storage_engines::sketch_db::accuracy::cms_params` — same. - `tests::accuracy_empirical_validation_tests` — six test fixtures migrated from `row_num` / `col_num` to `d` / `w` to match the helper's new contract. Remaining `row_num` / `col_num` mentions in the codebase are all internal Rust identifiers (function arguments / struct fields / locals describing the sketch matrix dimensions) — not config- parameter keys. Those are idiomatic and stay. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
Per-user direction: now that the asapcollector configs migrated to canonical
w/dkeys (sibling asapcollector PRsync-config-canonical-w-d), therow_num/col_numfallback PR #268 added has no remaining caller. Drop it.Three sweeps
accumulator_factory::cms_params— canonical keys only; renamed test fromcms_params_accepts_w_d_canonical_and_row_col_num_legacytocms_params_reads_canonical_w_d_keys, dropping the legacy half.storage_engines::sketch_db::accuracy::cms_params— same shape.tests::accuracy_empirical_validation_tests— six test fixtures migrated fromrow_num/col_numtod/wto match the helper's new contract.Remaining
row_num/col_nummentionsAll internal Rust identifiers (function arguments, struct fields, locals) describing the sketch matrix dimensions — not config-parameter keys. Those are idiomatic Rust and stay.
Test plan
cargo test --workspace --lib— 1534 pass, 0 failcargo test --test e2e_controller_plans_and_backend_serves— 10 passCross-repo ordering
This PR depends on the asapcollector
sync-config-canonical-w-dPR landing first (otherwise asapcollector deployments still shippingrow_num/col_numwould silently default to(4, 1000)). Merge order: asapcollector first, this PR second.🤖 Generated with Claude Code