perf(entity-resolution): exclude label entities from the trigram fuzzy-match index - #3214
Merged
Merged
Conversation
…y-match index (#3208) Label entities resolve by exact match only, yet their rows were still covered by the shared trigram index — every fuzzy probe for a regular entity pulled them into its candidate set only to discard them in the bitmap recheck. On banks where a free-text label group accumulated tens of thousands of mutually-similar values this dominated database CPU under ingest bursts. - Add entities.entity_kind ('regular'/'label', CHECK-constrained) on both dialects, set at insert time by the resolver; the Phase-2 reassert carries the kind so a pruned label parent resurrects as a label. - Rebuild the PG trigram index as a partial index excluding label rows (CONCURRENTLY, create-before-drop) and add the matching entity_kind != 'label' predicate to the trigram candidate query and the Oracle UTL_MATCH fuzzy scan. - Migration backfills existing rows per bank by classifying canonical_name against the bank's entity_labels config with the same is_label_entity() the resolver uses. - Fix the label classification gating on the enum lookup set: a config with only text/map groups builds an empty set, so its labels were never recognised — neither by the #3187 exact-lookup split nor by the new insert-time kind. Bank import needs no changes: transfer archives treat entities as derived data and re-resolve them through the standard retain Phase 1, which now stamps the kind.
nicoloboschi
force-pushed
the
feat/label-entities-kind-partial-trgm
branch
from
August 6, 2026 17:41
85ca3aa to
be0876a
Compare
This was referenced Aug 7, 2026
nicoloboschi
added a commit
that referenced
this pull request
Aug 7, 2026
…branch@-1 (#3238) `test_migration_drops_stale_global_index` stepped below the repair migration with `command.downgrade(cfg, "f2a6d8c4b1e9@-1")`. That is alembic's branch@relative syntax: it counts back from the *head* of the branch containing the revision, not from the revision itself. It meant "the parent" only for as long as f2a6d8c4b1e9 was head. b3e8d1c6f4a9 (#3214) landed on top of it, so `@-1` began resolving to f2a6d8c4b1e9 itself: the downgrade stopped ON the repair migration, the test planted the stale index after the drop had already run, and the upgrade never re-ran it. The test has been failing on main for every PR since — `assert 1 == 0` — with nothing wrong in the migration it covers. The parent now comes from the revision map, so the next migration added on top cannot break it. Verified it still fails when the DROP INDEX is disabled.
nicoloboschi
added a commit
that referenced
this pull request
Aug 7, 2026
`import_bank_async` resolved the target bank's config before the archive's bank row was restored. The bank cannot exist at that point — import refuses to write into an existing bank — so the resolve saw only global + tenant config and never the bank's own `entity_labels`, which arrives with the archive. Retain Phase 1 then classified every label entity as regular for the whole import. That silently disabled #3208/#3214 on imported banks: the partial trigram index `WHERE entity_kind != 'label'` excluded nothing, so every fuzzy probe kept paying the recheck-discard cost the index removes. It also let the import fuzzy-merge distinct label values, which the exact-match-only path (#3187) exists to prevent. The migration backfill does not cover it — it runs once, and a bank imported afterwards has nothing to correct it. Measured on an 862-document production export whose bank has a free-text label group: 5,374 label-shaped entities, of which the import marked 0 as labels. Correcting the classification takes entity-resolution p50 from 263 ms to 37 ms and a single fuzzy probe from 8.28 ms to 0.38 ms. import_bank now takes a `resolve_config` callback and re-resolves once the bank row is in place, replaying the documents with the bank's own config.
nicoloboschi
added a commit
that referenced
this pull request
Aug 7, 2026
…3237) `import_bank_async` resolved the target bank's config before the archive's bank row was restored. The bank cannot exist at that point — import refuses to write into an existing bank — so the resolve saw only global + tenant config and never the bank's own `entity_labels`, which arrives with the archive. Retain Phase 1 then classified every label entity as regular for the whole import. That silently disabled #3208/#3214 on imported banks: the partial trigram index `WHERE entity_kind != 'label'` excluded nothing, so every fuzzy probe kept paying the recheck-discard cost the index removes. It also let the import fuzzy-merge distinct label values, which the exact-match-only path (#3187) exists to prevent. The migration backfill does not cover it — it runs once, and a bank imported afterwards has nothing to correct it. Measured on an 862-document production export whose bank has a free-text label group: 5,374 label-shaped entities, of which the import marked 0 as labels. Correcting the classification takes entity-resolution p50 from 263 ms to 37 ms and a single fuzzy probe from 8.28 ms to 0.38 ms. import_bank now takes a `resolve_config` callback and re-resolves once the bank row is in place, replaying the documents with the bank's own config.
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.
Closes #3208.
Label entities (values of
entity_labelsconfig groups) resolve by exact match only, yet their rows were still covered by the shared trigram index — every fuzzy probe for a regular entity pulled them into its candidate set only to discard them in the bitmap recheck. On banks where a free-text label group accumulated tens of thousands of mutually-similar values, this recheck-discard overhead dominated database CPU under ingest bursts.What changed
entities.entity_kind('regular'/'label', CHECK-constrained, both dialects), set at insert time by the resolver — which already knows the classification. A kind column rather than a boolean so future entity kinds don't need another schema change. The Phase-2 reassert carries the kind onResolvedEntity, so a label parent pruned between phases resurrects as a label instead of silently re-entering the index.entities_canonical_name_lower_trgm_nonlabel_idx ... WHERE entity_kind != 'label'replaces the full index. BuiltCONCURRENTLY(autocommit block, invalid-leftover sweep, create-before-drop so fuzzy probes never lose coverage), skipped whenpg_trgmis absent. The trigram candidate query gains the textually-matchingAND e.entity_kind != 'label'predicate; the Oracle UTL_MATCH fuzzy scan gets the same filter (no index there, but the same wasted-scan semantics).canonical_names against that bank'sentity_labelsconfig using the sameis_label_entity()the resolver uses — no SQL reimplementation of the enum/text/map rules. Banks top out at 10–20k entities, so the synchronous per-bank backfill is fine. Label configs supplied only by a tenant extension can't be seen by the migration; those rows stayregular, which costs index size but never correctness.text/mapgroups builds an empty set, so its labels were never recognised — neither by the perf(entity-resolution): skip fuzzy probing for exact-match-only label entities #3187 exact-lookup split nor by the new insert-time kind. The gate is now on the config itself. Relatedly, thefullfallback strategy could fuzzy-merge a regular text into a label row (e.g.topic empathy→topic:empathyclears the 0.6 threshold with temporal proximity); the scoring loop now skips label-classified candidates, matching what the SQL probes enforce.Bank import needs no changes: transfer archives treat entities as derived data and re-resolve them through the standard retain Phase 1, which now stamps the kind. Admin backup/restore is likewise unaffected — restore COPYs by the backup's column list, so
entity_kindfills from its default for older backups.Expected impact
Tests
test_migration_entity_kind.py(dedicated pg0): backfill classifies exactly the configured label rows (enum + free-text groups, unconfigured banks untouched), partial index replaces the old one, downgrade restores the full index and drops the column.test_entity_resolver.py: end-to-end insert + reassert keepentity_kind='label'on a live PG; new unit test pins that fuzzy scoring never merges a regular text into a label row.test_entity_resolver_pg_trgm.py: the fuzzy probe SQL carries the partial-index predicate; the exact label lookup doesn't.Known tradeoff (documented in the docs + migration): the kind freezes classification at insert time. Removing a label group later leaves its ex-label rows out of the fuzzy index (exact match still works); rows created before a group existed stay in the index (overhead only).