Skip to content

[UoM prototype] Import measurement attribute vocabulary (2/4) - #951

Open
ricardotejedorsanz wants to merge 1 commit into
rt/uom-prototypefrom
rt/uom-measurement-vocabulary
Open

[UoM prototype] Import measurement attribute vocabulary (2/4)#951
ricardotejedorsanz wants to merge 1 commit into
rt/uom-prototypefrom
rt/uom-measurement-vocabulary

Conversation

@ricardotejedorsanz

@ricardotejedorsanz ricardotejedorsanz commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

This PR imports the Core/Admin-compatible V0 set of measurement attributes into the public taxonomy package.

⚠️ Reviewer note: taxonomy.json formatting

This PR also changes dist/en/taxonomy.json to be generated in compact JSON form. This is intentional, but it is not a trivial formatting change: after adding the measurement vocabulary, the pretty-printed generated taxonomy.json is ~106 MB, which exceeds GitHub's hard 100 MB per-file limit and cannot be pushed.

The compact file preserves the same JSON data contract for consumers that parse taxonomy.json, but it does make the file much less human-readable and creates a large formatting-only diff for that generated file. This PR still contains real taxonomy data additions; the formatting-only note applies specifically to the large line churn in dist/en/taxonomy.json. Reviewers should treat the large deletion count in dist/en/taxonomy.json as a consequence of compacting that generated file, not as deleted taxonomy content.

This is acceptable for prototype purposes, but we should still explore the right long-term distribution contract for large generated taxonomy artifacts.

What is included

This PR imports:

  • 1,235 public measurement attributes total
  • 33,219 category/measurement assignments
  • English names and descriptions for newly imported measurement attributes
  • regenerated dist/en files

Files changed

  • data/attributes.yml
    • adds the measurement attributes
    • each measurement attribute includes type: measurement, measurement_type, and supported_units
  • data/categories/*.yml
    • assigns the new measurement attributes to their relevant categories
  • data/localizations/attributes/en.yml
    • adds English names and descriptions for the new attributes so the English distribution can be generated
  • dist/en/attributes.json and dist/en/attributes.txt
    • regenerate the public English attribute distribution with measurement metadata
  • dist/en/categories.json and dist/en/taxonomy.json
    • regenerate category/taxonomy output so category attribute references include the new measurement attributes
  • dev/lib/product_taxonomy/commands/generate_dist_command.rb
    • writes taxonomy.json in compact JSON form so the generated taxonomy distribution stays below GitHub's file size limit as measurement coverage grows
  • dev/test/integration/all_data_files_import_test.rb
    • updates the expected Snowboards category attribute list now that measurement attributes are available there

Unit representation

For V0, supported_units contains canonical taxonomy unit symbols such as cm, in, ml, and fl oz.

Using symbols keeps the taxonomy data intentionally compact and compatible with other locales. The taxonomy should define the measurement vocabulary: which attributes exist, which categories they apply to, which measurement type they use, and which unit symbols are supported for each attribute.

This keeps the public taxonomy contract simple for V0. The remaining integration details—mapping taxonomy unit symbols to Core-supported units, validating supported units for each measurement type, and normalizing inferred or predicted measurement values before persistence—will be addressed next in Core as part of its own technical design.

Validation

  • bundle exec rake test:unit
  • bundle exec rake test:integration
  • bundle exec rake schema:vet

@@ -44,9 +44,15 @@ en:
absinthe_variety:
name: Absinthe variety
description: Differentiates absinthe products by their unique varieties, e.g. rouge, verte
absorbency_capacity:
name: Absorbency capacity

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Localization quality issue found

The following issues may affect the quality of localized translations if they are not addressed:

  • The value Absorbency capacity for key en.attributes.absorbency_capacity.name is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.

Please look out for other instances of this issue in your PR and fix them as well if possible.

Questions about these messages? Hop in the #help-localization Slack channel.

@@ -44,9 +44,15 @@ en:
absinthe_variety:
name: Absinthe variety
description: Differentiates absinthe products by their unique varieties, e.g. rouge, verte
absorbency_capacity:
name: Absorbency capacity
description: Absorbency capacity

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Localization quality issue found

The following issues may affect the quality of localized translations if they are not addressed:

  • The value Absorbency capacity for key en.attributes.absorbency_capacity.description is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.

Please look out for other instances of this issue in your PR and fix them as well if possible.

Questions about these messages? Hop in the #help-localization Slack channel.

@@ -161,6 +182,12 @@ en:
adc_resolution:
name: ADC resolution
description: Specifies the resolution (in bits) of the integrated Analog-to-Digital Converter, defining measurement precision
added_length:
name: Added length

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Localization quality issue found

The following issues may affect the quality of localized translations if they are not addressed:

  • The value Added length for key en.attributes.added_length.name is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.

Please look out for other instances of this issue in your PR and fix them as well if possible.

Questions about these messages? Hop in the #help-localization Slack channel.

@ricardotejedorsanz ricardotejedorsanz changed the title Import measurement attribute vocabulary [UoM prototype] Import measurement attribute vocabulary (2/3) Jul 1, 2026

@jeanguo-png jeanguo-png left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: since this PR compacted taxonomy.json to stay under GitHub's 100MB file limit, should we also add a small guard for categories.json? It is already ~95.7MB after this import, so the next assignment growth could hit the same limit.

@jeanguo-png jeanguo-png left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@danielpgross danielpgross left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the 100MB limit: we ran into the same challenge in the most recent release and our solution there was to move the file to LFS -- I think that's what we want here too. This isn't ideal long-term though, because it means the raw file can't be fetched without following an LFS pointer. Since these dist files are meant for easy consumption via API and the Github UI, we should think about a better long-term approach for that. But in the meantime, LFS is the way to go IMO.

As for the PR changes, my main question is about attributes like:

  • adapter_case_interior_height
  • bag_case_interior_height
  • bowling_pin_height
  • carpet_pile_height

Intuitively, I expected these to be extended attributes of a height base attribute, but I realized that they're actually each base attributes. I understand that this is necessary because they can support different units, but it still feels a bit incongruent with the existing attribute data model. I wonder if we can somehow make the base/extended attribute concept work here too. If not, should we somehow group all _height attributes together? Right now, they're just grouped in under dimension with others like width, length, depth, diameter, etc.

@ricardotejedorsanz

Copy link
Copy Markdown
Collaborator Author

Intuitively, I expected these to be extended attributes of a height base attribute, but I realized that they're actually each base attributes. I understand that this is necessary because they can support different units, but it still feels a bit incongruent with the existing attribute data model. I wonder if we can somehow make the base/extended attribute concept work here too. If not, should we somehow group all _height attributes together?

Good callout @danielpgross. I agree there is a semantic relationship between many of these height-like attributes, but I’m not sure extended attributes give us the same benefit for measurement attributes that they give us for closed-list attributes.

For closed-list attributes, the main benefit is inheriting the same controlled value set from the base attribute. For measurement attributes, the inherited contract is measurement_type and supported_units, which is useful for consistency but has less modeling leverage than sharing a controlled value list. It also only works cleanly when the concrete attribute and base attribute should share the same unit contract; some more granular measurement concepts may intentionally have different/narrower semantics or supported units than a generic base attribute.

For family-style groupings like _height, I’m not sure this is necessarily the right abstraction since they are already grouped at the measurement-type level like dimension or weight and this would add yet another layer with a less clear benefit to me.

@ricardotejedorsanz

Copy link
Copy Markdown
Collaborator Author

Non-blocking: since this PR compacted taxonomy.json to stay under GitHub's 100MB file limit, should we also add a small guard for categories.json? It is already ~95.7MB after this import, so the next assignment growth could hit the same limit.

Good point @jeanguo-png, Daniel noticed this too and is looking at it separately, so I’m going to keep that out of this MVP import PR (with a proposed fix is to stop storing generated dist files directly in git and instead publish them as artifacts attached to GitHub releases, including backfilling artifacts for existing releases).

@ricardotejedorsanz ricardotejedorsanz changed the title [UoM prototype] Import measurement attribute vocabulary (2/3) [UoM prototype] Import measurement attribute vocabulary (2/4) Jul 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants