Use effective family parameters for sketch measure identity

[robinld]

Problem

A t-digest metric that omits compression uses the default value of 200. Another metric can explicitly set compression to 200. Both generate the same sketch and the same component name.

Today, the first component is recorded with no parameters, while the second is recorded with compression=200. When the second metric tries to reuse the existing frozen measure, OSS rejects it because the recorded parameters differ.

Change

Let a registered family decomposition canonicalize parameters before OSS records them on a component. The t-digest implementation can treat an explicit default of 200 as no stored parameter, so it matches the existing sketch. A non-default value such as 500 remains distinct.

Metrics that already use an omitted default keep their existing empty-parameter identity.

Tests

The new regression test checks that omitted, integer, and integer-valued-float defaults produce the same component name and parameter identity. Local checks: 12 family-decomposition tests and 51 frozen-measure/preaggregation tests passed.

[netlify]

✅ Deploy Preview for thriving-cassata-78ae72 canceled.

Name	Link
🔨 Latest commit	b4c10c4
🔍 Latest deploy log	https://app.netlify.com/projects/thriving-cassata-78ae72/deploys/6abc51d66c99ee0008522f6d

[greptile-apps]

Confidence Score: 4/5

[Medium risk] Changes how sketch measure identity is computed in decomposition.

The PR should not merge until existing default-parameter frozen measures can be reused during metric redeployment.

Findings

1.  Existing sketch measures cannot be reused ▶

Summary

The PR stores a family decomposition’s effective parameters on extracted sketch components so equivalent declarations have the same measure identity.

• Adds a regression test for omitted, integer, and integer-valued-float defaults.
• Existing frozen measures created with omitted parameters need compatibility handling before the new identity can be used safely.

_{Reviews (1) · Last reviewed commit: "Use effective family parameters for sket..."}

[greptile-apps]

Existing sketch measures cannot be reused

If a metric with omitted sketch parameters already has a frozen measure, that measure was stored without parameters. This change extracts the same named component with effective parameters such as {"compression": 1}. Frozen-measure reuse compares those parameters and rejects the mismatch, so redeploying the metric fails instead of reusing its measure. Existing measures need a compatibility or migration path.

[robinld]

Good catch. Updated in b4c10c4: the family canonicalizes an explicit default to the legacy empty parameter map, rather than adding a default parameter to formerly unparameterized measures. The extractor only persists nonempty canonical parameters, so both omitted and explicit defaults retain params=None and reuse existing frozen measures. The regression test now asserts that identity for omitted, integer, and integer-valued-float defaults.

[datajunction-gh]

---

AI Review

Summary

The PR makes equivalent sketch declarations share a newly extracted component identity and preserves reuse of legacy omitted-default rows. It does not preserve reuse of rows previously stored with an explicit default, so existing metrics can fail on redeployment.

Findings

• ❌ datajunction-server/datajunction_server/sql/decompose.py:1886 A frozen measure created before this change with an explicit default retains its parameter map: for the registered test family, {"compression": 1}. Extracting the unchanged metric now strips that default and leaves component.params=None, while keeping the same component name and aggregation. Both individual and bulk derivation look up the existing row by name, then _raise_if_frozen_measure_conflicts rejects the unequal parameter maps. Redeploying that metric therefore fails. The prior discussion's fix covers rows created with an omitted default, not rows created with an explicit one.

  Provide an upgrade-safe equivalence check or migrate those stored rows before reuse. Stored preaggregations also retain parameter-bearing measure identity tokens, so account for their matching behavior. Add a regression starting from a pre-upgrade explicit-default row and exercising frozen-measure derivation.

Tests

• ⚠️ Redeployment through both individual and bulk frozen-measure derivation with a pre-upgrade, explicitly defaulted frozen row.
• ⚠️ Matching a stored preaggregation whose measure identity contains an explicit default against a newly canonicalized metric.
• ⚠️ An integration test with the production t-digest family; the repository test substitutes a family with a different default.

Missing context / blind spots

• ⚠️ The production t-digest family implementation is downstream of this repository, so its compression-200 normalization could not be verified.
• ⚠️ Focused tests could not run in this runtime because pytest and uv are unavailable; no test CI results were supplied.

Final Verdict

• Status: ❌ Block — do not approve
• Minimum required actions:
  • Preserve reuse of frozen measures created with an explicit default parameter, through a compatibility check or migration.
  • Account for stored preaggregation identities containing that explicit default, and test the upgrade path.

_{Reviewed commit b4c10c475dc2 · gpt-6-sol / xhigh via Netflix AgentCore githubcomreview/reviewer. Advisory automated review; no formal GitHub approval or request for changes.}

[datajunction-gh]

DataJunction deep review of b4c10c475dc26fe2af9150d5c11562e7bba136e1. Maintained summary includes off-diff findings. Advisory only.

[datajunction-gh]

A frozen measure created before this change with an explicit default retains its parameter map: for the registered test family, {"compression": 1}. Extracting the unchanged metric now strips that default and leaves component.params=None, while keeping the same component name and aggregation. Both individual and bulk derivation look up the existing row by name, then _raise_if_frozen_measure_conflicts rejects the unequal parameter maps. Redeploying that metric therefore fails. The prior discussion's fix covers rows created with an omitted default, not rows created with an explicit one.

Provide an upgrade-safe equivalence check or migrate those stored rows before reuse. Stored preaggregations also retain parameter-bearing measure identity tokens, so account for their matching behavior. Add a regression starting from a pre-upgrade explicit-default row and exercising frozen-measure derivation.