Skip to content

BRD-1150: add categoriesFlat field with split category levels - #76

Open
egidijuskunigonis wants to merge 3 commits into
mainfrom
brd-1150-categories-flat-field
Open

BRD-1150: add categoriesFlat field with split category levels#76
egidijuskunigonis wants to merge 3 commits into
mainfrom
brd-1150-categories-flat-field

Conversation

@egidijuskunigonis

@egidijuskunigonis egidijuskunigonis commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Synonym matching never reaches a leaf category today: the stored value is the whole path string, and a leaf like Sneakers can never match "Women > Shoes > Sneakers".

Each adapter now also emits a categoriesFlat_{locale} field: the unique level values of every category path, split on >. Every level becomes an independently-analyzed value that keyword/exact clauses can hit, and that synonym expansion can reach once the field carries the synonym search type (added in brad-app#538).

The field is search-only. categories and categoryDefault are byte-for-byte untouched, so filter aggregations and the storefront category tree do not change. Appending levels into categories itself was tested on a real index and rejected: level values dominate the top-50 facet buckets and pollute the filter sidebar — evidence on BRD-1150.

What changed

  • AdapterUtils::splitCategoryLevels() — shared helper: split on > (space-padded only, so a bare > inside a category name is not a delimiter), trim, dedupe preserving first-seen order.
  • PrestaShopAdapterV2 — emits categoriesFlat_{locale} per collected categories_{locale}. Covers PrestaShop and WooCommerce (brad-app routes WooCommerce through this adapter in V2).
  • MagentoAdapterV2 — same, from its hierarchical paths.
  • ShopifyAdapter — splits the taxonomy fullName only; tags are excluded because they are already flat single values. Emitted in locale mode only: ShopifyAdapter is the one adapter shared with the V1 sync path (brad-app's V1ProductTransformer calls transform() with no locales, which routes to buildPlainFields()), and the V1 bulk path posts the operation payload with no field whitelist. Keeping the field out of buildPlainFields() leaves V1 output byte-identical to main for every platform.

Example: ["Store > Summer > Men > T-Shirts", "Store > Spring > Men > Shirtlings"]categoriesFlat_{locale} = ["Store", "Summer", "Men", "T-Shirts", "Spring", "Shirtlings"].

Deploy notes (corrected after local end-to-end test)

  • Correction: the field goes over the wire on the first sync after the SDK bump. It is not gated by brad-app. An earlier revision of these notes claimed DataValidator makes the field inert until brad-app defines it. That is wrong on the path brad-app actually uses. DataValidator is constructed only by the V1 SynchronizationApiSdk (src/SynchronizationApiSdk.php:33), and it only validatesvalidateProduct() iterates the field configuration, never the product, so a key absent from the configuration is neither rejected nor stripped (src/Validators/DataValidator.php:24-58). The V2 path has no validator at all: Product::fromArray() sweeps every non-core key into additionalFields and jsonSerialize() writes them all straight back out (src/V2/ValueObjects/BulkOperations/Product.php:185-194,221-224), and brad-app's V2BulkOperations::indexProducts feeds it the stored payload verbatim.
  • It is still safe to ship ahead of brad-app#538, but for a different reason than previously given. Nothing in brad-search sets dynamic: strict on an index, so OpenSearch simply dynamically maps the extra key rather than rejecting the document. Until #538 lands, the field is absent from the pushed search configuration, and brad-search only ever queries the fields it finds in config.Fields (services/v2/search/query_builder_fields.go), so nothing searches, filters or facets it. It also receives OpenSearch's default dynamic mapping (text + .keyword with ignore_above: 256), not the intended analyzer chain — the field only becomes useful once #538's mapping preset is pushed and the index is rebuilt.
  • The transformation runs at collect time only. sync:reindex-applications and sync:resync-applications both replay stored product_sync_operations payloads without re-running the adapters — a reindex alone does NOT backfill this field. The data backfill happens via the first full platform sync after the SDK bump.
  • Read this before bumping the SDK: the first scheduled sync after the bump rewrites the whole catalogue of every V2 tenant. Checksums are computed over the transformed product (app/Services/ChangeDetectionService.php:16-21, fed the adapter output at app/Jobs/CollectProductDataJob.php:169,299), so adding a field changes the checksum of every product that belongs to at least one category — in practice the entire catalogue. sync:check-and-run is scheduled hourly and fires each application's own product_sync_cron, so this lands per tenant on its normal cadence, platform-wide, with no per-tenant rollout gate in front of it. Load to plan for: processProductForIncrementalSync goes from its documented "~5% of total" change rate to ~100%, i.e. one extra previous_data lookup query per product, one product_sync_operations UPDATE row per product carrying both the full previous and the full current payload, and a full re-push of every document to OpenSearch. Bump the SDK in a window where that is acceptable.
  • Verified locally end-to-end with brad-app#538: adapters emit correct levels, sidebar facets stay byte-identical, and exact-value leaf matching goes from impossible (categories.keyword == 'sneakers' → 0 docs) to working (categoriesFlat.keyword == 'sneakers' → 122 docs).
  • Correction: that keyword check demonstrates literal leaf matching, not synonym matching. An earlier revision of this line called it "the exact surface synonym matching uses", which is wrong. In the engine, synonym expansion happens only through the field's .synonyms subfield — the only place the synonym_graph filter lives is {lang}_synonym_search_analyzer (services/v2/mapping_builder.go:566-571), and that subfield is only queried for fields whose search types include synonym (models/query_config.go:101-104, services/v2/search/query_builder_fields.go:605-606). Exact clauses target .keyword/.keyword_ascii, whose normalizers cannot apply synonyms. brad-app#538 therefore gives categoriesFlat a synonym search type; without it this field would deliver literal leaf matching only and BRD-1150's acceptance criterion 2 could not pass.

Testing

  • 13 new tests: helper edge cases (multi-path dedupe, single-level path, spaced-delimiter-only splitting, whitespace, non-string values) plus per-adapter coverage asserting categoriesFlat_{locale} content and that categories/categoryDefault stay unchanged. ShopifyAdapterTest::testCategoriesFlatIsNotEmittedWithoutLocales pins the V1 / no-locale output to carrying no flat field at all.
  • Full suite green: 1615 tests, 4384 assertions. PHPStan level 4 clean, phpcs clean.
  • Pint not applied: the touched files already fail pint --test on main with the same fixers, so reformatting would add unrelated churn.

Confidence: 92%

Needs human verification

  • Synonym→leaf matching end-to-end needs a synonym configured on staging after the full rollout (SDK bump + brad-app presets + reindex + one full sync) — that check is BRD-1150's staging acceptance, not this PR. It also depends on brad-app#538 shipping the synonym search type on this field. Do not run that check on a Polish tenant: buildSynonymSubfield falls back to the plain search analyzer for languages with no stemmer/stop words, and Polish is currently the only such language.

Synonym and stemmed matching never reach a leaf category: synonyms need
an exact value match against the stored path string, and the engine's
analyzer only processes the first ~20 characters, so deep levels of
'Store > Summer > Men > T-Shirts' are invisible to search.

Each adapter now also emits categoriesFlat_{locale}: the unique level
values of every category path, split on ' > '. The field is search-only.
categories and categoryDefault are untouched, so filter aggregations and
the storefront category tree stay exactly as they are. Appending levels
into categories itself was tested and rejected: level values dominate
the top-50 facet buckets and pollute the filter sidebar (see BRD-1150).

PrestaShopAdapterV2 covers PrestaShop and WooCommerce; MagentoAdapterV2
splits its hierarchical paths; ShopifyAdapter splits the taxonomy
fullName only because tags are already flat single values.

The field only becomes searchable once brad-app adds it to the mapping
and search presets and applications reindex.
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@egidijuskunigonis
egidijuskunigonis marked this pull request as ready for review July 31, 2026 07:25
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

Move the new Magento tests above the helpers marker and drop a
narration comment from the Shopify test.
ShopifyAdapter is shared: brad-app's V1ProductTransformer calls
transform() with no locales, which routes to buildPlainFields(), and the
V1 bulk path posts the operation payload with no field whitelist. An
unsuffixed categoriesFlat there would hand a V1 tenant a field its index
never mapped, plus full checksum churn, for no benefit - brad-app's
presets define only categoriesFlat_{locale}.

Emit the field in locale mode only. V1 output is now byte-identical to
main for every platform.
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.

2 participants