OpenConceptLab/ocl_online#247 | Release-level vectorization: vectors for every semantic repo version, reused instead of re-encoded - #926
Conversation
…on, reused instead of re-encoded - A concept doc carries vectors when its repo's HEAD or any repo version the row belongs to is semantic, not only when HEAD is. So opting a release in gives its docs vectors, and rebuilding a row that a semantic release still uses (import, manual reindex, edit) keeps them. - Each vector records the text it encoded (`_embeddings.text`, `_synonyms_embeddings.text`: stored, not indexed) and each doc the model (`_embeddings_model`). A rebuild reuses a vector when the doc, or its versioned object's doc, already holds one for the same text from the same model; the other texts are encoded in one batched call per 100 docs. Docs written before this record neither, so their vectors are re-encoded when they're next rebuilt. - A `match_algorithms` change that adds or removes `llm` no longer rebuilds every doc of the version. It syncs the version's docs: those that need vectors and lack them are embedded, those whose vectors no semantic version (or HEAD) still uses are stripped, and the rest aren't touched. - New versions inherit vectorization: unless the request sets `match_algorithms`, a version is created with HEAD's, plus `llm` when HEAD or the latest released version is semantic. Indexing a new semantic version embeds its docs that have no vectors yet. - A locale change on a repo whose HEAD isn't semantic rebuilds the rows that carry vectors, so their display-name vector follows the new display name. - A change to both `released` and the reindex filters now queues both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRGfMH9ZR6TGNMSfiPyUaF
paynejd
left a comment
There was a problem hiding this comment.
Codex adversarial review, pass 1 (codex-cli 0.160.0, commit 47c611f)
-
High — core/concepts/embeddings.py:154: concurrent syncs can strip vectors a semantic version needs.
The semantic-membership check and unconditional full-document write are separate operations, without serialization or a final membership check.
Scenario: sync A prepares a vectorless document after the last semantic version opts out. Version B then opts in; its sync sees the existing vectors and skips that document. A subsequently writes its prepared document, stripping the vectors B needs. Both tasks succeed, leaving semantic search incomplete.
Fix: serialize semantic configuration changes and vector syncs per repository, and re-evaluate membership under that coordination before writing. Protect full-document writes against concurrent edits with ES sequence-number/primary-term checks and rebuild on conflict. Add an interleaving test covering opt-out preparation followed by opt-in. -
Medium — core/concepts/embeddings.py:47: search visibility is not a reliable vector-existence check.
get_ids_with_vectors()uses search, while sync itself writes withrefresh=False. The acknowledged refresh lag changes correctness, not just reporting.
Scenario: an opt-in sync fills vectors, then an opt-out sync runs before refresh. Search reports no vectors, so stripping is skipped permanently. Conversely, a second opt-in can rebuild already-vectorized documents—including legacy documents that cannot reuse their vectors—violating gap-fill behavior. The tests explicitly refresh before every sync, hiding this case.
Fix: inspect vectors through batched realtimemget, handling both dictionary and list embeddings. Add tests that run consecutive syncs without refresh. -
Medium — core/sources/models.py:536: new messages are incompatible with old workers during a rolling deploy.
Producers now enqueueindex_source_concepts(..., sync_vectors=True), but the previous task signature does not accept that keyword.
Scenario: the updated API handles a semantic flag change while an old indexing worker still consumes the queue. The task fails with an unexpected-keywordTypeError; the database flag changes, but vectors are never synchronized.
Fix: use a staged rollout that first deploys workers accepting the keyword, then enables producers, with queue draining or versioned routing to exclude old consumers. Document and test that compatibility sequence. -
Medium — core/concepts/documents.py:12: the new mapping requires an explicit deployment prerequisite.
Declaringtextas an unindexed keyword in Python does not update the existing production index. This change provides no mapping-update mechanism.
Scenario: an edit writes provenance before the mapping PUT. With dynamic mapping enabled, ES createstextas an indexed text field. A later PUT cannot change that field to keyword or disable its indexing, making the intended mapping impossible without replacing the index. With strict dynamic mapping, those writes fail instead.
Fix: apply and verify an additive mapping PUT for both nestedtextfields and_embeddings_modelbefore updated writers start. This can preserve existing documents without reindexing or re-embedding them.
Things checked and found correct:
- Python and SQL vector eligibility both include semantic HEAD and semantic M2M versions.
- Reuse requires matching model, explicit string text, and a non-null vector; legacy vectors without provenance are rejected.
- Versioned-object reuse remains keyed by exact text; duplicate texts, empty display text, and dictionary/list embeddings are handled.
- Chunk vectors resolve before actions are yielded, and temporary state is cleared in
finally. - Partial-update fallback and cache clearing remain present; inheritance checks HEAD and the latest released version.
- No added infrastructure identifiers or secrets were found.
Ran the requested diff. No files were modified and no network was used. Tests and pylint were not run because Docker socket access was denied.
…-in, see the last sync's writes, re-run by staff
Codex review, pass 1:
- A row whose need for vectors changes while its batch is written (another version opted in or out
meanwhile) is rebuilt once more. Before, an opt-out could strip a doc after the other version's own sync
had already checked it and found vectors (High).
- A sync refreshes the index first, so the docs the previous sync wrote (refresh=False) count (Medium).
- `POST .../{version}/concepts/indexes/` with `sync_vectors` (staff) queues only the vector sync, to re-run
a lost or failed one without changing the version's flag, e.g. one an old worker refused during a
rolling deploy (Medium).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRGfMH9ZR6TGNMSfiPyUaF
|
Codex pass 1: dispositions (fixed in 0391aa3)
|
paynejd
left a comment
There was a problem hiding this comment.
Codex adversarial review, pass 2 (codex-cli 0.160.0, commit 0391aa3)
-
High — HEAD sync omits historical rows affected by HEAD’s flag. core/concepts/embeddings.py:154
needs_vectorsand its SQL equivalent considerConcept.parent, but the sync enumerates only the version’s M2M members. Superseded rows lose HEAD membership (unmark_latest_version) while retaining HEAD as their parent.Failure: Create a lexical release, edit its concepts, then opt HEAD into
llm. The release’s superseded rows now need vectors under both eligibility functions, but HEAD’s sync never visits them. Conversely, opting HEAD out leaves unwanted vectors on those rows when their releases are lexical.Fix: For HEAD, enumerate
Concept.objects.filter(parent_id=version.id); for releases, retain the M2M selection. Add opt-in and opt-out tests using a superseded row retained in a lexical release. -
High — The corrective rebuild can still strip vectors a semantic version needs. core/concepts/embeddings.py:172
The post-write check repairs the original pass-1 interleaving, but its corrective write has no further concurrency protection.Failure:
- A fill prepares vectors while a version is semantic.
- That version opts out; its sync sees the still-missing vectors and skips the row.
- The original fill writes vectors, detects the opt-out, and prepares a corrective strip.
- Another version opts in; its sync sees the vectors and skips the row.
- The corrective strip writes last. The semantic version now has no vectors.
I reproduced this using the actual
sync_batchfunction with controlled eligibility and write interleavings. The new race test covers only the first write window and would catch the original bug, but not this one.Fix: Serialize reconciliation for overlapping versions of a repository, and include a final reconciliation under that serialization. Alternatively, use a generation/concurrency protocol that prevents stale corrective writes from winning. Add the interleaving above as a regression test; merely adding another unchecked rebuild moves the window.
-
Medium — The initial refresh does not eliminate visibility lag during a long sync. core/concepts/embeddings.py:166
Vector existence remains search-based. Refreshing once exposes preceding writes, but not writes occurring afterward while hundreds of thousands of rows are processed.Failure: Sync A starts and refreshes. Sync B writes vectors for a row in A’s later batches with
refresh=False. A still classifies that row as missing and rebuilds it, violating gap-fill behavior. If the intervening write contains legacy vectors without provenance, A also re-encodes those existing vectors.Fix: Read existence from realtime
mgetfor each batch, and protect the subsequent write against intervening changes through the concurrency mechanism above. The refresh regression test correctly covers pre-start writes and would fail without the refresh; it does not cover writes after sync starts.
Things checked and found correct:
- Reuse matches recorded text and model, rather than inferring text from the document’s current name. Legacy vectors without provenance are rejected.
- Own-document and versioned-object reuse, duplicate texts, empty display names, dict/list entries, and
Nonevectors do not reveal a text-reuse proof hole. - Chunk batching resolves vectors before yielding actions and clears
_vectorsafterward; retry generators recreate that state. - The staff recovery endpoint queues the intended sync arguments. It provides manual recovery for rolling-deploy failures, not automatic compatibility with old workers.
- Partial-update fallback, rejected-write handling, locale routing, cache clearing, and simultaneous release/flag queuing retain the intended behavior.
- The stored-only nested text mapping is appropriate with the stipulated mapping PUT deployment step. No new infrastructure identifiers or secrets appeared in the code diff.
No files were modified. Django/ES tests and pylint could not run: Docker socket access was denied, and the local Python environment lacks the required packages.
…sync repeats until the flags hold still Codex review, pass 2: - A doc carries vectors when any repo version the row belongs to is semantic, HEAD's membership included (Jon's call, 2026-10-02). That's exactly what a semantic $match can return, and what a HEAD sync visits. Before, a semantic HEAD meant every row of the repo, so its syncs missed superseded rows (High). - A sync ends each pass by comparing the match_algorithms of every version of the repo with what they were when the pass began, and runs again if any changed (up to 5 passes). This replaces the per-batch re-check, whose corrective rebuild could itself be overtaken by a later flag change (High). Each pass refreshes the index first. - Each sync request from a flag change carries a token of its own, so QueueOnce can't drop it as a duplicate of a sync of the same version that's already running. - Tests: superseded rows, HEAD opt-in filling HEAD's members only, a corrective strip followed by another opt-in, the pass limit; a small import (no `index` flag) and an edit keeping a semantic release's vectors. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRGfMH9ZR6TGNMSfiPyUaF
|
Codex pass 2: dispositions (fixed in 0ed51ec)
|
paynejd
left a comment
There was a problem hiding this comment.
Codex adversarial review, pass 3 (codex-cli 0.160.0, commit 0ed51ec)
-
High — core/concepts/embeddings.py:186: Equal flag snapshots do not prove flags stayed unchanged.
An off/on transition during one pass is invisible to this comparison. Concrete interleaving: a sync starts with a semantic version and existing vectors; the version opts out; the sync prepares a strip; the version opts back in; both newly queued syncs finish while vectors still exist and skip the doc; the delayed strip lands. The outer sync sees its original flags and returnssettled=True, leaving a semantic member without vectors after all queued syncs have run. I reproduced this using the actual function with in-memory dependencies.
Fix: Compare a durable, monotonically changing flag revision—or include reliably updated version timestamps—rather than flag values alone. Add an off/on test that actually runs the intervening syncs before the delayed write. -
High — core/concepts/embeddings.py:180: Exhausting the pass limit abandons reconciliation successfully.
On pass five, a flag can change after document preparation, and its queued sync can finish before the stale write lands. The stale writer then returnssettled=Falsewithout scheduling another pass or raising; the task succeeds with incorrect vectors and no remaining repair request. Unique tokens guarantee requests are accepted, not that a corrective request runs last. The new test attests_vectorization.py:431explicitly accepts this outcome.
Fix: On exhaustion, durably queue a uniquely tokened continuation after the current writes finish, or use an explicit retry mechanism. Test final vector correctness after draining every queued request. -
High — core/sources/models.py:557: A simultaneous full locale rebuild bypasses the concurrency protection.
When locale changes produce{}, the flag change gets neithersync_vectorsnor its unique token. For example, opting out while changing the default locale and supported-locales nullness queues a full rebuild. That rebuild can prepare a doc without vectors, then a later opt-in sync sees the existing vectors and finishes, after which the full rebuild strips them. Full indexing has no final flag check or corrective sync.
Fix: Preserve the required full locale rebuild, but always follow a flag-changing rebuild with a uniquely tokened reconciliation, or wrap the rebuild in the same revision-checked convergence mechanism. Simply addingsync_vectors=Trueto{}is insufficient: the task currently interprets that as sync-only. -
Medium — core/sources/models.py:545: Sync intent is not persisted before worker execution.
persist_args=Truesaves(source.id, None), butAsyncTask.apply_asyncpersists no kwargs;Task.kwargsis populated only inbefore_start. If a pending message is lost and recovered throughTask.rerun(force=True), its sync token is absent and it becomes a full reindex. Recovering a gap-fill request can therefore re-encode every legacy vector lacking provenance.
Fix: Persist the exact kwargs alongside args when enqueueing. Add a recovery test that reruns a task beforebefore_startand verifies it remains a sync request. -
Medium — core/common/tasks.py:690: The new messages are incompatible with old indexing workers.
During a mixed rollout, an old worker receivingsync_vectors=<token>fails with an unexpected-keyword error. An old worker receiving the compatible release-append message silently omits the new gap-fill step, leaving a newly inherited semantic release without vectors. No compatibility gate or ordered rollout mechanism accompanies the change.
Fix: Deploy and drain the indexing workers before enabling the new API behavior, with an explicit enforceable rollout procedure, or route the new behavior to a task/queue served exclusively by upgraded workers. -
Medium — core/concepts/documents.py:76: Existing indices do not acquire the declared mapping automatically.
The diff supplies no additive mapping installation or guard before writers emit the new fields. Without the mapping PUT, ordinary dynamic mapping creates the provenance strings as searchabletextfields with keyword subfields. A subsequent PUT cannot convert them to the intended source-only keyword mapping; correcting the production index would require rebuilding it. With restrictive dynamic mappings, writes can fail instead.
Fix: Install_embeddings.text,_synonyms_embeddings.text, and_embeddings_modeladditively before enabling upgraded writers, without population/reindexing. Verify the installed definitions and test against an index created with the previous mapping.
Things checked and found correct:
prepare(),needs_vectors, SQL membership checks, and locale classification consistently use actual M2M membership, including HEAD.- Reuse requires recorded text and matching model; legacy vectors are excluded. A stale reuse-source document does not invalidate that proof by itself.
- Empty text, duplicate texts, dict/list embeddings, and null vectors are handled appropriately.
- Batch vectors resolve before actions are yielded. Class-declared state works with DSL attribute assignment, and retries recreate the generator.
- The normal semantic release-append path performs gap-fill; shared semantic members retain vectors on rebuild.
- The
elif→ifchange correctly permits release-state indexing and vector reconciliation together. - No new infrastructure identifiers or security-sensitive details appeared in the diff.
Read-only review completed; no files changed and no network used. New-module local lint and diff whitespace checks passed. Django/ES tests and coverage could not run because Docker access was denied.
…cks itself later; a mapping command for the deploy Codex review, pass 3. The in-sync repairs (passes until the flags hold still) could each be outrun: an opt-out and back in looks unchanged, the pass limit gave up, and a full locale reindex skipped the sync. - A semantic flag change, a semantic release's indexing and the staff `sync_vectors` option queue `sync_source_concept_vectors(source_id, recheck=True)`: one sync now, and one more VECTOR_SYNC_RECHECK_SECONDS (600) later, also after a failure. Anything prepared from the flags as they were before the change has been written by then, and the recheck repairs it (High x2). - persist_changes queues the sync whenever the flag changes, beside any locale reindex, including a full one; get_concepts_reindex_filters is about locales again (High). - The task's arguments are positional and persisted, so a rerun of a lost task is still a sync (Medium). - index_source_concepts keeps its signature, so old and new workers both take its messages; the new task is the only thing an old worker can't run (Medium, documented in the PR). - `python manage.py concept_vector_mapping [--check]` adds the three fields to an existing concepts index, additively, and checks them; tested against an index with the old mapping (Medium). - The pass loop, its limit and the per-request tokens are gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRGfMH9ZR6TGNMSfiPyUaF
|
Codex pass 3: dispositions (fixed in 2f4b235) Findings 1–3 share a cause: each repair ran inside the sync, so another write could outrun it. The sync now repairs from outside, after a delay.
|
…in no version yet count as HEAD's Codex review, pass 4: - seed_children_to_new_version queues the sync (and its recheck) for every semantic version it seeds: whether or not it's still the latest release, which is the only case its indexing covers, and however that indexing goes. index_source_concepts no longer queues it after the append (High, Medium). - A row in no repo version yet counts as its HEAD's. A new concept is saved, and indexed, before it joins HEAD (concept creation isn't atomic), so a write prepared before that could land after the one prepared after it and strip a semantic HEAD's new docs. Now both carry vectors (High). Superseded rows only in a lexical release still don't; rows in no version keep a semantic HEAD's vectors, as before #247. - The staff `sync_vectors` option goes through Source.sync_concept_vectors_async, so its arguments are persisted like every other sync's (Medium). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRGfMH9ZR6TGNMSfiPyUaF
paynejd
left a comment
There was a problem hiding this comment.
Codex adversarial review, pass 4 (codex-cli 0.160.0, commit 2f4b235)
-
High —
core/common/tasks.py:713: semantic releases that finish seeding out of order can receive no vector sync.
Sync is reachable only after an_append_source_versionindexing task. However, seeding a released version callsindex_resources_for_self_as_latest_released()(core/common/tasks.py:415), which does nothing unless that version is still the latest release (core/sources/models.py:451).Failure: lexical HEAD has unvectorized concepts. Create semantic release v1, then explicitly lexical release v2 before v1’s seed finishes. V1 subsequently gains its members but queues neither its append indexing nor its vector sync. V2’s lexical indexing doesn’t embed them. Every queued task can finish successfully while v1’s members lack vectors; the 600-second assumption offers no repair because no recheck exists.
Fix: always queue membership indexing and a sync/recheck after a semantic version is seeded, independently of whether it remains the latest release. Keep latest-release flag updates conditional. Add an out-of-order seeding test.
-
High —
core/concepts/documents.py:256: new semantic HEAD memberships have an unrepaired overwrite race.
Vector preparation now depends on M2M membership, but concept construction saves rows before attaching them to HEAD (core/concepts/models.py:1121,1156; edits similarly save before1229). Those saves can queue indexing while the row has no semantic membership. The subsequent membership signal queues another full write, but neither path queues a delayed repair.Failure: with HEAD already semantic, an early save task prepares a document without vectors. Construction adds HEAD membership; its signal task writes the correctly vectorized document. The earlier write then lands last, stripping the vectors. Both writes can finish within seconds, satisfying the stated delay assumption, and every queued task has run.
Fix: schedule a repair/recheck when semantic membership is added, preferably scoped or coalesced to avoid a source-wide sync per edit. Alternatively, make construction atomic and defer all indexing until commit. Add a test that reverses the landing order of the pre-membership and post-membership writes.
-
Medium —
core/common/tasks.py:713: a failed release append prevents both the sync and its recheck.
The new sync scheduling sits afterindex_concepts(), outside its failure cleanup.Failure: a new semantic release has already been seeded. Its append pass successfully updates most documents but exhausts retries on one rejected batch.
index_concepts()raises, so no sync is queued. Successfully appended documents still lack vectors, even after ES recovers. The recovery sweep only selects strandedSTARTEDtasks, not ordinaryFAILUREtasks (core/common/tasks.py:1086).Fix: queue the semantic membership sync/recheck even when append indexing fails, while preserving the original indexing exception and summary. Add a rejected-append test asserting that repair is still scheduled.
-
Medium —
core/sources/views.py:375: the staff sync endpoint does not persist its task arguments.
This endpoint usesIndexingTaskMixin.post(), whoseapply_async()call omitspersist_args=True(core/tasks/mixins.py:66). UnlikeSource.sync_concept_vectors_async(), it therefore persists no(source_id, recheck)arguments.Failure: staff requests
sync_vectors; the worker dies during execution. Recovery callsTask.rerun(), which substitutes()for missing arguments. The recovered task fails with missingsource_id, so neither repair nor recheck runs. The endpoint tests only inspect dispatched arguments and miss this.Fix: persist arguments in this scheduling path, or route it through the source helper. Test the stored
Task.argsand actual recovery dispatch. -
Medium —
core/sources/models.py:556: the new task is incompatible with old indexing workers during a rolling deployment.
New API instances publishsync_source_concept_vectorsonto the existing shared indexing queue. Pre-PR workers do not register that task.Failure: an old worker consumes an opt-in sync and discards it as an unknown task. Its body never executes, so its
finallycannot schedule the recheck. The persisted task remains pending, outside the stranded-STARTEDrecovery sweep. Separately, old full-index workers still strip release-only vectors because they check HEAD alone.Fix: require a worker-first deployment with old indexing workers drained before enabling new producers, or isolate the new task on a queue consumed exclusively by upgraded workers. Include this alongside the mapping prerequisite in the deployment procedure.
Things checked and found correct:
- Python and SQL membership predicates implement “any semantic membership,” including HEAD.
- Reuse requires matching stored model and exact entry text; legacy provenance-free vectors and
Nonevectors are rejected. Dict/list entries, duplicate texts, and empty display text are handled. - Stale stored documents do not themselves defeat reuse proof: reuse is keyed by recorded text rather than current document name.
- Declared document state attributes avoid the inspected
AttrDictassignment trap; chunk state is cleared before yielding. - The initial refresh and delayed recheck cover the tested flag-flip races under the stated write-delay assumption.
- The mapping addition is additive and performs no reindex. No new infrastructure identifiers or secrets appeared in the diff.
No files were modified or network accessed. Syntax checks passed. Django/ES tests and coverage could not run because Docker access was denied; local lint was limited by missing project dependencies.
|
Codex pass 4: dispositions (fixed in 74a339d)
|
…r seeding Codex review, pass 5 (High): seed_children_to_new_version decided whether to queue the vector sync from the flag as it was when the task started. An opt-in while seeding ran queued a sync, and its recheck, that could finish before any of the version's members existed, and then nothing queued another. It now re-reads match_algorithms after seeding. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VRGfMH9ZR6TGNMSfiPyUaF
paynejd
left a comment
There was a problem hiding this comment.
Codex adversarial review, pass 5 (codex-cli 0.160.0, commit 74a339d)
-
High — core/common/tasks.py:415: Seeding checks a stale semantic flag and can permanently miss vectorization.
instanceis loaded at line 385, before snapshot/checksum work and membership seeding. The laterhas_semantic_match_algorithmcheck still uses that original value.Failure scenario: A lexical version starts seeding. While lengthy checksum work runs, staff opt it into
llm. Its immediate sync and 600-second recheck both finish before concept memberships exist. Seeding then completes, but the cached lexical flag prevents another sync. Existing ES docs receive only the release’s partial update, leaving them without vectors after every queued task finishes. This satisfies the stated prepared-write timing assumption: the delay is before membership creation, not between document preparation and writing.Fix: Reload
match_algorithmsafter seeding, before deciding whether to queue the sync. Alternatively, queue a sync unconditionally after seeding and let it evaluate current state. Add a regression test that changes the stored flag and runs both opt-in syncs before allowing seeding to continue.
Checked and found correct through source inspection:
- Python and SQL vector eligibility include semantic version membership and the no-membership HEAD fallback.
- Reuse requires recorded text and matching model; legacy vectors without provenance are excluded.
- Batched encoding deduplicates identical texts, and generator state is cleared before yielding actions.
- Opt-out preserves vectors shared with another semantic version; failed initial syncs still schedule rechecks.
- The mapping command is additive; the diff exposes no infrastructure identifiers or secrets.
git diff --checkpassed, and all changed Python files parsed successfully.
No files were modified and no network was used. Docker access was denied, so ES-backed tests, pylint, and coverage remain unverified. PR comments were unavailable offline.
|
Codex pass 5: dispositions (fixed in bd0e9b4)
|
paynejd
left a comment
There was a problem hiding this comment.
Codex adversarial review, pass 6 (codex-cli 0.160.0, commit bd0e9b4): clean
No significant findings in origin/master...HEAD at bd0e9b47. I found no remaining path that leaves a semantic version’s member without vectors after successful queued syncs, under the stated 600-second write assumption.
Checked and found correct:
- Seeding re-reads
match_algorithmsafter populating membership and queues sync even for a release that is no longer latest. - Python and SQL vector eligibility rules agree, including the no-membership creation fallback.
- Reuse requires exact recorded text and model; legacy vectors without provenance are rejected. Versioned-object reuse follows the same checks.
- Sync refreshes before gap detection, preserves shared vectors, and schedules its recheck even after failure.
- Imports, edits, manual reindexes, and locale rebuilds use membership-aware preparation.
- New-version inheritance, explicit overrides, staff-only sync access, persisted task arguments, and the unchanged indexing-task signature are consistent.
- Mapping changes are additive; deploying alone triggers no reindex. The command explicitly requires applying the mapping before provenance-writing code.
- No infrastructure identifiers or security-sensitive details appeared in the diff.
Verification: changed Python files parsed successfully, and git diff --check passed. Files remained unchanged. Docker access was denied and local dependencies were unavailable, so tests, pylint, coverage, ES behavior, and SQL plans could not be verified at runtime.
| Queues this version's vector sync (sync_source_concept_vectors) on the indexing queue, with its arguments | ||
| persisted, so that a rerun of the task is still a sync, and returns its Task. In TEST_MODE it runs inline. | ||
| """ | ||
| task = Task.new(queue='indexing', user=user or self.updated_by, name=sync_source_concept_vectors.__name__) |
There was a problem hiding this comment.
task is only used in TEST_MODE=False move it into else condition and return None in if branch
|
|
||
| from core.common.utils import encode_texts | ||
|
|
||
| SYNC_BATCH_SIZE = 500 |
There was a problem hiding this comment.
this constant must be reused in common/models.py#batch_index_full as well
| needing = get_concept_ids_needing_vectors(batch) | ||
| with_vectors = get_ids_with_vectors(index_name, batch) | ||
| fill = [_id for _id in batch if _id in needing and _id not in with_vectors] | ||
| not_needing = [_id for _id in batch if _id not in needing] |
There was a problem hiding this comment.
faster better set subtraction (I think its ok to ignore the order in the batch of 500)
needing_set = set(needing)
with_vectors_set = set(with_vectors)
batch_set = set(batch)
fill = (batch_set & needing_set) - with_vectors_set
not_needing = batch_set - needing_set
| """ | ||
| STORED_FIELDS = ['_embeddings', '_synonyms_embeddings', '_embeddings_model'] | ||
|
|
||
| def __init__(self, index_name): |
There was a problem hiding this comment.
index_name must be hardcoded here -- to prevent cross reference from caller between Django model and ES index
| while chunk := list(islice(objects, self.VECTOR_CHUNK_SIZE)): | ||
| self._vectors = ConceptVectors(self._index._name) | ||
| try: | ||
| actions = [self._prepare_action(obj, action) for obj in chunk if self.should_index_object(obj)] |
There was a problem hiding this comment.
can use actions = self.get_actions(chunk, action)
| texts = [str(text) for text in texts] | ||
| if not texts: | ||
| return [] | ||
| if settings.ENV == 'ci': |
There was a problem hiding this comment.
this check should be before texts = [str(text) for text in texts]
| if not model: | ||
| from sentence_transformers import SentenceTransformer | ||
| model = SentenceTransformer(settings.LM_MODEL_NAME) | ||
| return list(model.encode(texts, batch_size=settings.LM_ENCODE_BATCH_SIZE)) |
There was a problem hiding this comment.
wouldn't want to mutate texts input value -- not sure how the caller wants to reuse it
def get_lm_model():
model = settings.LM
if not model:
from sentence_transformers import SentenceTransformer
model = SentenceTransformer(settings.LM_MODEL_NAME)
return model
def encode_texts(texts):
"""Embeddings for several texts, from one batched model call (OpenConceptLab/ocl_online#247)."""
if not texts or not isinstance(texts, (set, list, str)):
return []
if settings.ENV == 'ci':
return [None] * len(texts)
model = get_lm_model()
return list(model.encode([str(text) for text in texts], batch_size=settings.LM_ENCODE_BATCH_SIZE))
| """ | ||
| if not versions_match_algorithms: | ||
| return bool(parent.has_semantic_match_algorithm) | ||
| return any(parent.SEMANTIC_MATCH_ALGORITHM in (algorithms or []) for algorithms in versions_match_algorithms) |
There was a problem hiding this comment.
this can be flattened
from pydash import flatten, compact
return parent.SEMANTIC_MATCH_ALGORITHM in compact(flatten(versions_match_algorithms))
| """ | ||
| match_algorithms = list(self.match_algorithms or [self.TOKEN_MATCH_ALGORITHM]) # as clean_match_algorithms | ||
| if self.SEMANTIC_MATCH_ALGORITHM not in match_algorithms: | ||
| latest_released = self.get_latest_released_version() |
There was a problem hiding this comment.
just pointing out that this is the first thing we are copying from latest released version of a repo.
In scenario where a repo version and HEAD were semantic available and we want to stop newer versions -- we will have to remove llm in match_algorithms from HEAD and latest released version of the repo -- removing from HEAD was good -- which will make last released version also semantic unavailable.
This creates a different path from -- v1 (prev latest) is semantic available but v2 (latest) is not.
| texts = [str(text) for text in texts] | ||
| if not texts: | ||
| return [] | ||
| if settings.ENV == 'ci': |
There was a problem hiding this comment.
should we also add "demo" env check here -- it was overridden in settings.py by making it default constant
Linked Issue
Closes OpenConceptLab/ocl_online#247. The ocl-cli criterion is in OpenConceptLab/ocl-cli#12. LOINC's own releases are verified separately, in OpenConceptLab/ocl_online#363.
Summary
Until now only HEAD's
match_algorithmsproduced vectors. Opting a release in to semantic search marked it semantic but wrote no vectors. Any flag change rebuilt every doc of the version and re-encoded every name, one string perencodecall, even when the vectors already existed. A rebuild of rows shared with a vectorized release (an import, a manual reindex, an edit) also stripped their vectors whenever HEAD wasn't semantic.This PR makes vectors follow the semantic repo versions, reuses them instead of re-encoding them, and makes new versions inherit vectorization.
Which docs carry vectors
A concept doc carries vectors when any repo version the row belongs to is semantic, HEAD's membership included. That's exactly the set of docs a semantic
$matchcan return, since it always searches one version's members. Before, "HEAD semantic" meant every row of the repo.sourcesquery per doc, shared withsource_version, so documents need no extra queries.Reuse
_embeddings.text,_synonyms_embeddings.text, stored but not indexed). Each doc records the model (_embeddings_model).ConceptDocument._get_actionsprepares 100 docs at a time. It reads their stored vectors, and those of their versioned objects' docs, in onemget.search_index.Opting a version in or out
A
match_algorithmschange that adds or removesllmno longer rebuilds every doc of the version. It queues a vector sync:sync_source_concept_vectors, a task of its own on the indexing queue, runningsync_concept_vectors. Seeding a new semantic version queues one too. The sync:It checks 500 rows at a time, with one SQL query and one or two ES queries per batch, after refreshing the index. A failed batch fails the task. The task summary adds
filled,stripped,vectors_reusedandtexts_encoded.Concurrent changes. A doc is prepared from the flags as they are at that moment, then written. A doc prepared just before another change can therefore land after that change's own sync has checked it.
VECTOR_SYNC_RECHECK_SECONDSlater (default 600), even if the first run failed. By then, anything prepared from the old flags has been written, and the recheck repairs it.POST .../{version}/concepts/indexes/with{"sync_vectors": true}.New versions (decision V1)
Unless the create request sets
match_algorithms, a new version gets HEAD's list, plusllmwhen HEAD or the latest released version is semantic. A request that sets them wins, so an owner can opt a release out. Seeding a semantic version queues its vector sync, which embeds its docs that have no vectors yet. That happens whether or not the version is still the latest release, and whatever happens to its indexing.Other paths
releasedchange, a locale reindex and a vector sync each queue their own task. Before, areleasedchange skipped the locale reindex.LM_MODEL_NAMEis always set, because the docs record it.LM_ENCODE_BATCH_SIZE(default 64) sets the encode batch.VECTOR_SYNC_RECHECK_SECONDS(default 600) sets the recheck delay.python manage.py concept_vector_mapping [--check]adds the new fields to an existingconceptsindex mapping, additively, and checks them (see Notes).Test Plan
core/concepts/tests/tests_vectorization.py: 49 tests against the test Elasticsearch, with a deterministic stand-in for the model. They cover:encodeper chunk;_seq_no);index=true, and a small one under the default), a manual reindex and an edit, each keeping a semantic release's vectors;sync_vectorsoption, with its arguments persisted;encode_textsunit tests, plus updatedget_concepts_reindex_filtersexpectations.--parallel=1 --keepdb,PYTHONHASHSEED=2, fresh indexes): 2,452 tests OK, coverage 95% (CI gate 95%).pylint -j0 core/: exit 0.Measurement
The same script ran against
origin/masterand this branch. It used 1,000 concepts fromcore/samples/ciel-2020-09-27.ndjson(2,000 HEAD docs: latest rows and versioned objects) and the realall-MiniLM-L6-v2, and counted every string the model encoded. It calls the tasks a PATCH or a reindex queues, synchronously. Strings encoded are deterministic. Timings are from a laptop running other work, so treat them as indicative.masterA1 encodes about half as many strings as
master. A concept's versioned-object and latest rows share their names, and each text is encoded once per chunk.Notes
python manage.py concept_vector_mappingagainst each environment'sconceptsindex before its deploy. It's an additive mapping change with no reindex, and it then checks the result (--checkonly checks). It adds:{"_embeddings": {"type": "nested", "properties": {"text": {"type": "keyword", "index": false, "doc_values": false}}}, "_synonyms_embeddings": {"type": "nested", "properties": {"text": {"type": "keyword", "index": false, "doc_values": false}}}, "_embeddings_model": {"type": "keyword"}}textpluskeywordthe first time a doc carries them, which indexes every name inside every nested vector doc, and the command can no longer apply. Fresh indexes (CI,search_index --create) get the mapping from the document.$matchcan return them. Nothing strips them at deploy; their vectors are dropped when such a row is next rebuilt. Rows in no version at all keep a semantic HEAD's vectors, as before.index_source_conceptskeeps its signature.sync_source_concept_vectorsis new, so an old indexing worker can't run it. Avoidmatch_algorithmsPATCHes during the rollout.sync_vectorsoption above.