fix(import): classify label entities when restoring a bank (#3236) - #3237
Merged
Conversation
`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
force-pushed
the
fix/import-label-entity-kind
branch
from
August 7, 2026 10:42
8f30aa1 to
1ce8f5b
Compare
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.
Fixes #3236.
Found while benchmarking #3208/#3214 against a real production export.
The bug
import_bank_asyncresolved the target bank's config beforeimport_bankrestored the archive's bank row:
The bank cannot exist at that point — import explicitly refuses to write into an
existing bank — so that resolve only ever sees global + tenant config. The bank's
own
entity_labels, which the archive carries inbanks.json, arrives later. RetainPhase 1 then classified every label entity in the imported bank as
regular.Consequences:
WHERE entity_kind != 'label'excluded nothing, so every fuzzy probe kept payingthe recheck-discard cost the index exists to remove.
(perf(entity-resolution): skip fuzzy probing for exact-match-only label entities #3187) keys off the same classification, so distinct label values could collapse
into one another (Memory Tags added when Entities are not during retain #1558).
afterwards has nothing to correct it.
The fix
import_banknow takesresolve_config(a callback) instead of a resolved config,and calls it right after the bank row is restored. This is deliberately not an
optional extra parameter alongside
config: the only correct moment to resolve isinside
import_bank, so making it the only way to supply one keeps the next callerfrom reintroducing the bug.
import_bank_asyncis the sole caller.Measured on a real export
862 documents / 38,162 facts, bank has a free-text
brief_biolabel group. Importedinto an emptied instance (all other banks dropped — the trigram index has no
bank_id, so a leftover bank pollutes any measurement here):label18.1× less entity-resolution work than the baseline and 14.7× less than main as
shipped — on this bank the defect was hiding nearly the whole benefit of #3214.
(The 500
labelrows visible on unfixed main in the issue came from a post-importingest burst, not the import. A post-hoc backfill classified 5,355; the fixed import
produces 5,719 because the broken path had fuzzy-merged some distinct label values
into each other.)
Test
test_bank_import_classifies_label_entities: configureentity_labels, export,delete, import, assert the label entity returns as
entity_kind='label'and aregular one as
'regular'. Verified it fails onmainwith'regular' != 'label'.test_document_transfer.py,test_admin_bank_transfer.py,test_repair_bank_vector_indexes.py,test_hermes_templates_import.py,test_entity_resolver.pyandtest_entity_labels.pypass;lint.sh+tyclean.