Skip to content

OpenConceptLab/ocl_online#247 | Release-level vectorization: vectors for every semantic repo version, reused instead of re-encoded - #926

Open
paynejd wants to merge 6 commits into
masterfrom
ocl_online-247-release-vectorization
Open

paynejd wants to merge 6 commits into
masterfrom
ocl_online-247-release-vectorization

Conversation

@paynejd

@paynejd paynejd commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

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_algorithms produced 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 per encode call, 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 $match can return, since it always searches one version's members. Before, "HEAD semantic" meant every row of the repo.

  • A row in no version yet counts as its HEAD's. Creating a concept saves and indexes the row before adding it to HEAD, so this keeps those writes in step with the one after, whichever lands last.
  • Rows that are only in lexical releases carry no vectors.
  • The check is one sources query per doc, shared with source_version, so documents need no extra queries.

Reuse

  • Each vector now records the exact text it encoded (_embeddings.text, _synonyms_embeddings.text, stored but not indexed). Each doc records the model (_embeddings_model).
  • ConceptDocument._get_actions prepares 100 docs at a time. It reads their stored vectors, and those of their versioned objects' docs, in one mget.
  • A vector is reused only when its recorded text and model match exactly. Every other text is encoded in one batched call per chunk. Every full-index path goes through this: batch indexing, save signals, imports, manual reindexes and search_index.
  • Docs written before this change record neither text nor model, so their vectors are never reused. Their next rebuild re-encodes them, as it does today, but in batches.

Opting a version in or out

A match_algorithms change that adds or removes llm no 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, running sync_concept_vectors. Seeding a new semantic version queues one too. The sync:

  • embeds the docs that need vectors and lack them (gap-fill);
  • strips the docs that carry vectors that no version they belong to still needs;
  • leaves every other doc untouched, so opting a version out never strips vectors that another semantic version shares.

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_reused and texts_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.

  • So every sync a change queues runs once more VECTOR_SYNC_RECHECK_SECONDS later (default 600), even if the first run failed. By then, anything prepared from the old flags has been written, and the recheck repairs it.
  • The task's arguments are positional and persisted, so a rerun of a lost task is still a sync.
  • Staff can re-run a sync without touching the flag: 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, plus llm when 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

  • Locale changes. On a repo whose HEAD isn't semantic, a locale change still updates only the name fields, except for rows in a semantic version. Those are rebuilt, with reuse, so their display-name vector follows the new display name.
  • One PATCH changing several things. A released change, a locale reindex and a vector sync each queue their own task. Before, a released change skipped the locale reindex.
  • Settings. LM_MODEL_NAME is 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.
  • Deploy aid. python manage.py concept_vector_mapping [--check] adds the new fields to an existing concepts index mapping, additively, and checks them (see Notes).

Test Plan

  • New core/concepts/tests/tests_vectorization.py: 49 tests against the test Elasticsearch, with a deterministic stand-in for the model. They cover:
    • which docs carry vectors, including superseded rows and rows in no version yet;
    • reuse, and no reuse across a model change or from docs that don't record their text;
    • reuse from the versioned object's doc;
    • one batched encode per chunk;
    • opt-in gap-fill, leaving shared docs unwritten (unchanged _seq_no);
    • opt-out keeping the vectors another semantic version uses;
    • HEAD opt-in filling HEAD's members only;
    • a flag change landing between a batch's prepare and its write, both across versions and as an opt-out and back in, each repaired by the recheck;
    • the sync task: its delayed recheck, also after a failure, and its persisted arguments;
    • a flag change together with a full locale reindex queueing both;
    • a sync seeing writes made after the last refresh;
    • seeding a semantic version queueing its sync (also when it's no longer the latest release, or was opted in while seeding ran), and that sync filling it;
    • an import (index=true, and a small one under the default), a manual reindex and an edit, each keeping a semantic release's vectors;
    • the locale path;
    • the staff sync_vectors option, with its arguments persisted;
    • V1 inheritance through the API, including a non-staff member and an explicit opt-out;
    • the mapping command, against an index created with the old mapping, including a conflicting field.
  • encode_texts unit tests, plus updated get_concepts_reindex_filters expectations.
  • Full suite locally (--parallel=1 --keepdb, PYTHONHASHSEED=2, fresh indexes): 2,452 tests OK, coverage 95% (CI gate 95%).
  • pylint -j0 core/: exit 0.
  • Before/after embedding work on a CIEL sample with the real model (below).
  • PR CI: Pylint and Tests pass on bd0e9b4
  • Codex adversarial review: 6 passes posted on this PR, each followed by its dispositions. Pass 6 is clean.

Measurement

The same script ran against origin/master and this branch. It used 1,000 concepts from core/samples/ciel-2020-09-27.ndjson (2,000 HEAD docs: latest rows and versioned objects) and the real all-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.

Step master this branch
A1: HEAD opted in (flag flip) 4,736 strings in 4,736 calls, 171 s 2,393 strings in 20 batched calls, 18 s
A2: release opted in while HEAD is semantic rebuilds the release: 2,368 re-encoded, 89 s 0 encoded: every doc already has vectors (0.1 s)
A3: manual reindex of HEAD 4,736 re-encoded, 169 s 0 encoded: all reused, 15 s
A4: manual reindex after 10 names changed 4,736 re-encoded, 147 s 10 encoded, 15 s
B1: release opted in while HEAD is lexical (the LOINC case) 0 of 500 docs get vectors 500 of 500, 1,110 strings
B2: manual reindex of HEAD, whose rows that release shares 0 of 500 500 of 500 kept, 0 encoded

A1 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

  • Add the mapping before deploying. Run python manage.py concept_vector_mapping against each environment's concepts index before its deploy. It's an additive mapping change with no reindex, and it then checks the result (--check only 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"}}
    Without it, ES maps them dynamically as text plus keyword the 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.
  • Deploying writes nothing to existing vectors. Release indexing and gap-fill leave docs that already have vectors alone. Docs written before this change are re-encoded only when a full rebuild touches them.
  • Rows only in lexical releases, such as a semantic HEAD's superseded rows kept in an old lexical release, no longer need vectors. No semantic $match can 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.
  • Rolling deploy. index_source_concepts keeps its signature. sync_source_concept_vectors is new, so an old indexing worker can't run it. Avoid match_algorithms PATCHes during the rollout.
    • Versions created or flipped during the window may miss their sync. Re-run it with the staff sync_vectors option above.
    • A semantic version seeded by an old worker gets no sync. Re-run its sync the same way. Until the rollout completes, old workers embed only under a semantic HEAD, so keep the interim LOINC rules until then.

…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 paynejd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Codex adversarial review, pass 1 (codex-cli 0.160.0, commit 47c611f)

  1. 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.

  2. 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 with refresh=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 realtime mget, handling both dictionary and list embeddings. Add tests that run consecutive syncs without refresh.

  3. Medium — core/sources/models.py:536: new messages are incompatible with old workers during a rolling deploy.
    Producers now enqueue index_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-keyword TypeError; 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.

  4. Medium — core/concepts/documents.py:12: the new mapping requires an explicit deployment prerequisite.
    Declaring text as 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 creates text as 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 nested text fields and _embeddings_model before 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
@paynejd

paynejd commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

Codex pass 1: dispositions (fixed in 0391aa3)

  1. High: concurrent syncs can strip vectors another version needs. Fixed.
    • After a batch's rebuild, sync_concept_vectors re-checks which of those rows need vectors, from the DB. Any row whose need changed while the batch was prepared and written is rebuilt once more.
    • That closes the interleave: if the other version's flag change commits after this batch was prepared but before its write, the re-check sees it. If it commits later, that version's own sync checks the doc after the write.
    • Test: test_opting_out_while_another_version_opts_in_keeps_the_vectors_that_version_needs. It commits the opt-in between prepare and write.
    • Not covered: an unrelated writer (an edit's save signal) that prepared before a flag change and writes after that version's sync. That's the same prepare-to-write window every derived field has today. The post-release flag/vector check (spec rule 6) is tracked separately.
  2. Medium: the vector check relies on search visibility. Fixed. A sync refreshes the index before its first check, so the previous sync's refresh=False writes count.
    • I kept search (a nested exists query, ids only) over a realtime mget. An mget would have to fetch the vectors to know they're present: megabytes per 500 rows on the largest versions.
    • Test: test_sync_sees_vectors_written_since_the_last_refresh. It writes with refresh disabled, then syncs.
  3. Medium: new messages reach old workers during a rolling deploy. Mitigated. No new message type is safe for an old worker.
    • The deploy notes say: no match_algorithms PATCH during the rollout.
    • If a sync is lost anyway, staff can now re-run it without touching the flag: POST .../{version}/concepts/indexes/ with {"sync_vectors": true}. Tests: SourceConceptsIndexViewSyncVectorsTest.
  4. Medium: the mapping is a deploy prerequisite. Already a deploy step (PR notes): the additive mapping goes on each environment's concepts index before its deploy, and is checked with GET concepts/_mapping/field/....

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Codex adversarial review, pass 2 (codex-cli 0.160.0, commit 0391aa3)

  1. High — HEAD sync omits historical rows affected by HEAD’s flag. core/concepts/embeddings.py:154
    needs_vectors and its SQL equivalent consider Concept.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.

  2. 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_batch function 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.

  3. 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 mget for 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 None vectors do not reveal a text-reuse proof hole.
  • Chunk batching resolves vectors before yielding actions and clears _vectors afterward; 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
@paynejd

paynejd commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

Codex pass 2: dispositions (fixed in 0ed51ec)

  1. High: a HEAD sync omits superseded rows that the parent rule says need vectors. Fixed by changing the rule. A doc now carries vectors when any version the row belongs to is semantic, HEAD's membership included. A semantic $match always searches one version's members, so that's exactly the set it can return, and exactly what each sync visits.
    • The maintainer chose this over making HEAD syncs walk every row of the repo. On a large repo, most of those rows sit in no semantic version, so their vectors would be unreachable.
    • Tests: test_superseded_row_under_a_semantic_head_has_no_vectors_unless_a_semantic_version_holds_it and test_opting_head_in_fills_its_members_only.
  2. High: the corrective rebuild can be overtaken by a later flag change. Fixed by replacing the per-batch re-check with passes.
    • Each pass ends by comparing the match_algorithms of every version of the repo with what they were when it began. If any changed, the sync runs again, up to 5 passes (settled in the summary says whether the last pass saw no change).
    • Docs are prepared from the flags as they are at that moment. So the last pass that saw no change wrote every doc it touched from the current flags. Any write prepared from older flags belongs to a pass that saw the change and is followed by another pass.
    • Each sync request from a flag change now carries its own token, so QueueOnce can't drop it as a duplicate of a running sync of the same version. Before, that same-version case fell through.
    • Your interleaving is now test_a_late_corrective_strip_cannot_win_over_a_later_opt_in: fill, opt-out before the fill lands, strip, opt-in before the strip lands. It ends with vectors after 3 passes. test_sync_gives_up_after_its_last_pass_while_flags_keep_changing covers the limit.
    • I chose this over a lock. Production's API and most workers reach Postgres through transaction-pooled pgbouncer, where a session advisory lock can leak, and a long sync would hold any lock for hours.
  3. Medium: visibility lag during a long sync. Not changed, on these grounds:
    • Every flag change gets its own sync, and every sync re-runs until its flags settle. So the writes a long sync can't see yet come only from edits and imports.
    • Those writes record text and model whenever they carry vectors. The worst case is one redundant rewrite of such a doc, with every vector reused and nothing re-encoded.
    • Legacy vectors are never written afresh, so they can't appear mid-sync.
    • A realtime mget would have to fetch the vectors themselves to see them: megabytes per 500 rows on the largest versions.

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Codex adversarial review, pass 3 (codex-cli 0.160.0, commit 0ed51ec)

  1. 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 returns settled=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.

  2. 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 returns settled=False without 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 at tests_vectorization.py:431 explicitly 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.

  3. High — core/sources/models.py:557: A simultaneous full locale rebuild bypasses the concurrency protection.
    When locale changes produce {}, the flag change gets neither sync_vectors nor 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 adding sync_vectors=True to {} is insufficient: the task currently interprets that as sync-only.

  4. Medium — core/sources/models.py:545: Sync intent is not persisted before worker execution.
    persist_args=True saves (source.id, None), but AsyncTask.apply_async persists no kwargs; Task.kwargs is populated only in before_start. If a pending message is lost and recovered through Task.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 before before_start and verifies it remains a sync request.

  5. Medium — core/common/tasks.py:690: The new messages are incompatible with old indexing workers.
    During a mixed rollout, an old worker receiving sync_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.

  6. 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 searchable text fields 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_model additively 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 → if change 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
@paynejd

paynejd commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

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.

  1. High: equal flag snapshots don't prove the flags held still. Fixed. The snapshot comparison is gone. Every sync that a change queues (sync_source_concept_vectors(source_id, recheck=True)) runs once more VECTOR_SYNC_RECHECK_SECONDS later (default 600).
    • Each change gets its own delayed recheck: an opt-out, an opt-in, a semantic release's indexing, a staff re-run.
    • A doc prepared from the flags as they were before the change is written within a batch's prepare-to-write window, seconds rather than minutes, so it has landed by then.
    • The recheck's own writes are prepared after the change.
    • Your interleaving is test_an_opt_out_and_back_in_during_a_sync_is_repaired_by_the_recheck: opt-out, strip prepared, opt-in, and that sync runs and skips, then the strip lands. The recheck fills the doc again.
    • Assumption: a write lands within 10 minutes of being prepared. If one doesn't, or a recheck message is lost, the post-release flag/vector check (spec rule 6) is the backstop, and the staff sync_vectors re-run repairs it.
  2. High: an exhausted pass limit abandons reconciliation. Fixed: there's no pass limit any more. A failed sync still queues its recheck (test_a_failed_sync_still_queues_its_recheck).
  3. High: a full locale rebuild bypasses the sync. Fixed. persist_changes queues the sync whenever the semantic flag changes, beside any locale reindex, including a full one (test_flag_flip_with_a_full_locale_reindex_queues_both). get_concepts_reindex_filters is about locales again.
  4. Medium: sync intent isn't persisted. Fixed. The sync is its own task with positional (source_id, recheck) arguments, queued with persist_args=True, so a rerun of a lost task is still a sync (test_a_queued_sync_keeps_its_arguments_for_recovery).
  5. Medium: messages old workers can't take. Reduced and documented.
    • 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. The PR notes say: no match_algorithms PATCH during the rollout, and re-run the sync for any version created or flipped in the window.
    • An ordered rollout (indexing worker first) is a pipeline decision. The deploy plan carries it.
  6. Medium: existing indices don't get the mapping. Fixed as a deploy step with a tool. python manage.py concept_vector_mapping adds the three fields to an existing concepts index, additively, then checks them; --check only checks.
    • Tests run against an index created with the old mapping, including a field already mapped differently, which is refused.
    • The deploy runs it before the code that writes the fields.

…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 paynejd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Codex adversarial review, pass 4 (codex-cli 0.160.0, commit 2f4b235)

  1. 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_version indexing task. However, seeding a released version calls index_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.

  2. 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 before 1229). 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.

  3. Medium — core/common/tasks.py:713: a failed release append prevents both the sync and its recheck.
    The new sync scheduling sits after index_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 stranded STARTED tasks, not ordinary FAILURE tasks (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.

  4. Medium — core/sources/views.py:375: the staff sync endpoint does not persist its task arguments.
    This endpoint uses IndexingTaskMixin.post(), whose apply_async() call omits persist_args=True (core/tasks/mixins.py:66). Unlike Source.sync_concept_vectors_async(), it therefore persists no (source_id, recheck) arguments.

    Failure: staff requests sync_vectors; the worker dies during execution. Recovery calls Task.rerun(), which substitutes () for missing arguments. The recovered task fails with missing source_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.args and actual recovery dispatch.

  5. Medium — core/sources/models.py:556: the new task is incompatible with old indexing workers during a rolling deployment.
    New API instances publish sync_source_concept_vectors onto 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 finally cannot schedule the recheck. The persisted task remains pending, outside the stranded-STARTED recovery 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 None vectors 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 AttrDict assignment 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.

@paynejd

paynejd commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

Codex pass 4: dispositions (fixed in 74a339d)

  1. High: a semantic release seeded out of order gets no sync. Fixed. seed_children_to_new_version now queues the sync, with its recheck, for every semantic version it seeds. That holds whether or not the version is still the latest release, which is the only case its indexing covers. Test: test_seeding_a_semantic_version_queues_its_sync (v1 seeded after v2 was released).
  2. High: new semantic HEAD members race their pre-membership writes. Fixed with a narrower rule rather than a sync per edit. A row in no repo version yet counts as its HEAD's.
    • Concept creation isn't atomic: the row is saved, and indexed, before sources.set([parent]). Every write prepared before the membership now agrees with the one after, so landing order no longer matters.
    • Edits and imports already index after commit (save_as_new_version pauses indexing; ImportIndexer runs after the part), so they didn't race.
    • Superseded rows that are only in a lexical release still carry no vectors. Rows in no version at all keep a semantic HEAD's vectors, as before FHIR Bundle links #247.
    • The SQL predicate has the same clause. Tests: test_a_row_in_no_version_yet_counts_as_its_heads and test_superseded_row_under_a_semantic_head_has_vectors_only_if_a_semantic_version_holds_it.
  3. Medium: a failed append skips the sync and its recheck. Fixed by #1: the sync is queued by the seed task, no longer after the append.
  4. Medium: the staff option doesn't persist its arguments. Fixed. The endpoint goes through Source.sync_concept_vectors_async, which persists (source_id, recheck). Test: test_sync_vectors_persists_its_arguments reads the stored Task.args.
  5. Medium: old indexing workers during a rolling deploy. Procedural, as before. The deploy plan:
    • updates the indexing worker before the API where the pipeline allows it;
    • keeps the interim LOINC rules until the rollout completes, since old workers still embed only under a semantic HEAD;
    • re-runs the sync afterwards for every version created or flipped in the window.

…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 paynejd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Codex adversarial review, pass 5 (codex-cli 0.160.0, commit 74a339d)

  1. High — core/common/tasks.py:415: Seeding checks a stale semantic flag and can permanently miss vectorization.
    instance is loaded at line 385, before snapshot/checksum work and membership seeding. The later has_semantic_match_algorithm check 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_algorithms after 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 --check passed, 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.

@paynejd

paynejd commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

Codex pass 5: dispositions (fixed in bd0e9b4)

  1. High: seeding checks a stale semantic flag. Fixed. seed_children_to_new_version re-reads match_algorithms after seeding, before deciding whether to queue the sync. An opt-in that lands while seeding runs therefore gets a sync after its members exist. Test: test_seeding_reads_the_flag_after_seeding, which flips the stored flag mid-seed.

@paynejd paynejd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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_algorithms after 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.

Comment thread core/sources/models.py
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__)

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.

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

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.

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]

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.

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):

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.

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)]

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.

can use actions = self.get_actions(chunk, action)

Comment thread core/common/utils.py
texts = [str(text) for text in texts]
if not texts:
return []
if settings.ENV == 'ci':

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.

this check should be before texts = [str(text) for text in texts]

Comment thread core/common/utils.py
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))

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.

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)

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.

this can be flattened

from pydash import flatten, compact
return parent.SEMANTIC_MATCH_ALGORITHM in compact(flatten(versions_match_algorithms))

Comment thread core/sources/models.py
"""
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()

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.

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.

Comment thread core/common/utils.py
texts = [str(text) for text in texts]
if not texts:
return []
if settings.ENV == 'ci':

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.

should we also add "demo" env check here -- it was overridden in settings.py by making it default constant

This branch has not been deployed

No deployments
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