Bulk embeddings pool for ingest (remediates #2282)

[JSv4]

Summary

This carries #2282 forward: ingest embeddings can go to a separate autoscaled microservice pool while search queries stay on the warm one. It is configured only through the PipelineSettings singleton. #2282 is merged in here onto current main and fixed.

The blocking bug: #2282 was cut before utils/embedding_identity.py landed. embedding_configuration() hashes every non-secret setting on MicroserviceEmbedder.Settings, so adding embeddings_microservice_url_bulk (even with its empty default) would change the fingerprint of every existing microservice vector on deploy. valid_embeddings filters on that fingerprint, so existing vectors would drop out of readiness and search. The existing test_billing_setting_does_not_change_vector_identity_or_legacy_fingerprint fails on the merged #2282 code for exactly this reason.

Changes

• utils/embedding_identity.py: the bulk URL is removed from the fingerprint, like no_external_provider_fees. It only controls routing; the bulk pool must serve the same model.
• MicroserviceEmbedder._get_service_config: same routing as Add bulk embeddings pool for ingest (via the pipeline settings singleton) #2282 (use_bulk_pool=True plus a configured bulk URL goes to the bulk pool; anything else goes to the query URL), with less code.
• Budgeted runs (embed_text_accounted) deliberately stay on the query URL, which run_policy.resolve_provider validates and pins as the endpoint. Routing them elsewhere would bypass that check.
• Tests: the three _get_service_config tests are now one table test. I removed TestIngestBulkPoolRouting and test_batch_tags_bulk_pool, which repeated assertions the updated existing tests already make. Added test_bulk_pool_url_does_not_change_vector_identity.
• Docs (performance_tuning.md): existing installs should set the bulk URL in System Settings. I dropped Add bulk embeddings pool for ingest (via the pipeline settings singleton) #2282's migrate_pipeline_settings --force advice: that command resets settings that have no env var (embedding_model_revision, no_external_provider_fees), which breaks budgeted runs. Comments, sample env notes and the changelog fragment are trimmed.

Test plan

Ran against local Postgres + pgvector:

pytest test_batch_embedding.py test_embeddings_task.py test_dual_embeddings.py \
       test_ingestion_run_microservice.py test_migrate_pipeline_settings_command.py \
       test_pipeline_settings_schema.py test_ingestion_readiness.py
285 passed, 76 subtests passed

With the embedding_identity.py fix reverted, the existing legacy-fingerprint test and the new identity test both fail. The full backend suite also passes in CI.

Checklist

• Tests pass locally for any code this PR touches
• pre-commit run passes on changed files (black, isort, flake8, mypy, changelog check)
• TypeScript compiles cleanly: n/a, no frontend changes
• A changelog fragment was added under changelog.d/
• No new dependencies

Contributor License Agreement

By submitting this pull request, you agree to license your contribution under the project's Contributor License Agreement.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CoR99QxfqaXZFesYRGcBiC

[claude]

Code Review

Reviewed the bulk-embeddings-pool routing changes (MicroserviceEmbedder._get_service_config, embeddings_task.py call-site updates, embedding_identity.py fingerprint exclusion, docs, and changelog fragment). Core routing logic is correct — the bulk-pool URL override is resolved before Cloud Run auth is computed, and the new field is correctly excluded from the vector-identity fingerprint (opencontractserver/utils/embedding_identity.py:39-40), scoped to only the MicroserviceEmbedder path where the field actually exists.

Two issues found, one docs/ops risk and one latent code gap:

1. Docs recommend a command with a broader blast radius than described

docs/deployment/performance_tuning.md:201 tells operators to run:

python manage.py migrate_pipeline_settings --component MicroserviceEmbedder --force

to seed the new EMBEDDINGS_MICROSERVICE_URL_BULK setting. Per migrate_pipeline_settings.py (lines 213-303), --force resets every setting on the component to its env-var/default value, not just the new field. embedding_model_revision and no_external_provider_fees (opencontractserver/pipeline/embedders/sent_transformer_microservice.py) have no env_var in their PipelineSetting metadata, so this exact command silently blanks any operator-configured embedding_model_revision (breaks policy-bound ingestion per its own docstring) and resets no_external_provider_fees back to False (re-includes self-hosted infra in run-budget accounting), regardless of whether those were set via the System Settings UI. Worth a caveat in the doc, or scoping the recommended command more narrowly.

2. Multimodal ingest path not updated to use the bulk pool

opencontractserver/utils/multimodal_embeddings.py:329 (generate_multimodal_embedding) calls embedder.embed_text(raw_text) without use_bulk_pool=True, unlike the three ingest call sites updated in opencontractserver/tasks/embeddings_task.py (lines 78, 634, 1105). This is inert today since no multimodal embedder (multimodal_microservice.py) currently defines embeddings_microservice_url_bulk, but it's an incomplete migration — if that field is later added to a multimodal embedder (the natural extension of this PR's pattern), image-bearing annotation ingest would silently stay on the query pool while text-only ingest moves to bulk, undermining the load-isolation goal for exactly the annotations most likely to be expensive. No existing test would catch this (the multimodal fallback tests only exercise the text-only fallback path).

Minor / non-blocking

• sent_transformer_microservice.py:257: if all_kwargs.get("use_bulk_pool") and bulk_url: relies on raw truthiness rather than bool(...) (contrast use_cloud_run_iam_auth, which is explicitly wrapped). Harmless today since all call sites pass literal True, but embed_text/embed_texts_batch accept **kwargs with no type enforcement, so a future caller passing a truthy-but-unintended value would silently route to bulk.
• No guard against a whitespace-only EMBEDDINGS_MICROSERVICE_URL_BULK value — not a regression from this PR, but the new field inherits the same gap as the existing URL field.

Positives

• Test coverage is solid: test_get_service_config_routes_only_flagged_calls_to_bulk_pool covers all three routing branches, and test_bulk_pool_url_does_not_change_vector_identity confirms the fingerprint-exclusion behavior directly.
• Changelog fragment and docs follow the repo's pointer-based conventions (code references instead of pasted snippets, correct <slug>.<type>.md naming).
• No magic numbers, no duplicated "pool"/alternate-URL abstraction reinvented elsewhere in the codebase.

[JSv4]

Thanks, I checked each point against the code.

1. --force blast radius: Confirmed. embedding_model_revision, no_external_provider_fees and use_cloud_run_iam_auth have no env_var, and the command has no way to force a single setting. Fixed in 36098f9: the docs now say to set the field in System Settings (the form is built from the settings schema, so the new field shows up there), and warn against --force.
2. Multimodal path: Leaving this as is. generate_multimodal_embedding only runs when embedder.is_multimodal and embedder.supports_images. MicroserviceEmbedder is neither, and it is the only embedder with a bulk URL. Tagging that call now would be a speculative change with no effect. If a multimodal embedder later gets a bulk URL, that change should tag this call site as well.
3. Minor points: Leaving as is. Every caller passes a literal True. A blank URL is handled the same way as the existing embeddings_microservice_url.

---

Generated by Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!