From a946b26434606c6b9b2447d22c3d0d9de2fdd4e0 Mon Sep 17 00:00:00 2001 From: Aakash Kattelu Date: Thu, 13 Aug 2026 10:13:33 -0700 Subject: [PATCH] chore: drop ticket ids and shrink comments to one sentence Comments and docstrings describe current behavior, not the PR that introduced them. Ticket numbers stay in the commit/PR. --- src/deriver/deriver.py | 3 --- src/embedding_client.py | 19 +++---------------- src/exceptions.py | 7 +------ src/reconciler/sync_vectors.py | 5 ----- tests/crud/test_representation_manager.py | 2 +- tests/deriver/test_deriver_processing.py | 5 +---- tests/live_llm/README.md | 2 +- tests/live_llm/test_live_embeddings.py | 6 +----- 8 files changed, 8 insertions(+), 41 deletions(-) diff --git a/src/deriver/deriver.py b/src/deriver/deriver.py index a139e4f5..f76c4d52 100644 --- a/src/deriver/deriver.py +++ b/src/deriver/deriver.py @@ -344,9 +344,6 @@ async def process_representation_tasks_batch( ) ) - # If every observer's save failed, surface the failure to the queue manager so the - # work unit is marked errored instead of silently processed with zero documents saved - # (#728). Raised after telemetry so metrics still record the attempt. if save_errors and successful_observer_count == 0: details = "; ".join( f"{observer}: {exc.__class__.__name__}: {exc}" diff --git a/src/embedding_client.py b/src/embedding_client.py index f89d1fcb..aeecae09 100644 --- a/src/embedding_client.py +++ b/src/embedding_client.py @@ -318,15 +318,7 @@ class _EmbeddingClient: ) def _truncate_to_token_limit(self, text: str) -> tuple[str, int]: - """Return a prefix of `text` whose re-encoded token count fits the cap. - - `decode(ids[:n])` can re-encode to more than `n` tokens at BPE / - pre-tokenizer boundaries (a slice landing mid-character decodes to - replacement chars that cost tokens of their own), so we re-encode - after each slice and trim again until the verified count fits. Each - retry keeps strictly fewer tokens than the last, so the loop always - terminates instead of oscillating around the cap. - """ + """Return a prefix of `text` whose re-encoded token count fits the cap.""" token_ids = self.encoding.encode(text) keep = self.max_embedding_tokens while len(token_ids) > self.max_embedding_tokens: @@ -335,8 +327,6 @@ class _EmbeddingClient: return "", 0 text = self.encoding.decode(token_ids[:keep]) token_ids = self.encoding.encode(text) - # Guarantee forward progress: if this slice still re-encodes over - # the cap, the next one must be strictly smaller. keep -= 1 return text, len(token_ids) @@ -354,11 +344,8 @@ class _EmbeddingClient: Args: texts: List of text strings to embed - on_oversize: What to do when an input exceeds `max_embedding_tokens`. - ``"raise"`` (default) preserves the historical contract. - ``"truncate"`` embeds a token-capped prefix and keeps the - one-input → one-vector mapping, so one long item cannot drop - the rest of the batch. + on_oversize: ``"raise"`` (default) errors; ``"truncate"`` embeds a + token-capped prefix. Returns: List of embedding vectors, one per input text (in order) diff --git a/src/exceptions.py b/src/exceptions.py index 3df7815d..e4d7009c 100644 --- a/src/exceptions.py +++ b/src/exceptions.py @@ -135,12 +135,7 @@ class VectorStoreError(HonchoException): @final class RepresentationSaveError(HonchoException): - """Raised when every observer's representation save fails in a batch. - - Surfaced from the deriver to the queue manager so the work unit is marked - errored instead of silently processed with zero documents saved. The - underlying save exception is preserved as the cause via ``raise ... from``. - """ + """Raised when every observer's representation save fails in a batch.""" status_code = 500 detail = "Representation save failed for all observers" diff --git a/src/reconciler/sync_vectors.py b/src/reconciler/sync_vectors.py index 8f510f47..8bf8d74d 100644 --- a/src/reconciler/sync_vectors.py +++ b/src/reconciler/sync_vectors.py @@ -301,11 +301,6 @@ async def _sync_documents( EmbeddingCallPurpose.VECTOR_SYNC.value, parent_category="reconciliation", ): - # Document content is stored whole, so an observation can be - # over the per-item embedding cap (the write path truncates the - # embedded prefix, not the stored text). Truncate here too: - # raising would drop every other document in this batch on - # every reconciler pass. new_embeddings = await embedding_client.simple_batch_embed( contents, on_oversize="truncate" ) diff --git a/tests/crud/test_representation_manager.py b/tests/crud/test_representation_manager.py index fa60aa69..3c2f57e4 100644 --- a/tests/crud/test_representation_manager.py +++ b/tests/crud/test_representation_manager.py @@ -636,7 +636,7 @@ class TestRepresentationManagerSave: @pytest.mark.asyncio async def test_save_representation_embeds_with_truncate_on_oversize(self): - """One oversize observation must not drop the rest of the batch (#569).""" + """One oversize observation must not drop the rest of the batch.""" manager = RepresentationManager( "workspace", observer="observer", diff --git a/tests/deriver/test_deriver_processing.py b/tests/deriver/test_deriver_processing.py index 0bc58d40..29a983b5 100644 --- a/tests/deriver/test_deriver_processing.py +++ b/tests/deriver/test_deriver_processing.py @@ -73,10 +73,7 @@ class TestDeriverProcessing: assert "llm_settings" not in kwargs async def test_all_observer_saves_failing_surfaces_failure(self): - """When every observer's save_representation fails, the batch must raise - instead of swallowing it. Without this the work unit is marked processed - with zero documents saved: silent memory loss (#728). - """ + """When every observer's save_representation fails, the batch must raise.""" message = Mock( id=1, public_id="msg_1", diff --git a/tests/live_llm/README.md b/tests/live_llm/README.md index cdf248c2..32a6ddd4 100644 --- a/tests/live_llm/README.md +++ b/tests/live_llm/README.md @@ -68,5 +68,5 @@ Coverage by provider: - OpenAI transport → OpenRouter non-reasoning models (e.g. `inception/mercury-2`): non-chat / diffusion architectures must stay on `max_tokens`, no `reasoning_effort`, tool-calling parameter-schema compatibility is the canary for exotic OR-served providers - Gemini 2.5/3.0 classes: structured outputs, cached-content reuse, thought signatures, multi-turn tool replay - Gemini 3.1 class: thinking and tool replay coverage by default; structured-output/caching coverage should only be added once Google documents support for that path -- Embeddings (`test_live_embeddings.py`): single embed, batched embed, batch-vs-single alignment, chunk-to-id mapping, and oversize-truncate survival (`on_oversize="truncate"`, #569) for every configured embedding model. `gemini-embedding-2*` is the reason this exists — those models collapse a list of bare strings into one document (#745), and only a live call catches it +- Embeddings (`test_live_embeddings.py`): single embed, batched embed, batch-vs-single alignment, chunk-to-id mapping, and oversize-truncate survival (`on_oversize="truncate"`) for every configured embedding model. `gemini-embedding-2*` is the reason this exists — those models collapse a list of bare strings into one document (#745), and only a live call catches it - OpenAI-compatible embedding providers (e.g. OpenRouter's `google/gemini-embedding-001`): the #932 surface. Those providers reject a base64 embedding request outright (HTTP 400) or answer HTTP 200 with empty data, so the whole matrix fails without `encoding_format="float"`. Real OpenAI accepts base64 happily, so only a third-party provider catches it. Note that OpenRouter load-balances across upstreams, so the base64 failure is per-attempt rather than guaranteed: a retry can land on an endpoint that accepts it. `test_live_openai_float_encoding_matches_base64` covers the other side, that the float switch must not move vectors on real OpenAI diff --git a/tests/live_llm/test_live_embeddings.py b/tests/live_llm/test_live_embeddings.py index 3567abc5..e8e16be6 100644 --- a/tests/live_llm/test_live_embeddings.py +++ b/tests/live_llm/test_live_embeddings.py @@ -151,11 +151,7 @@ async def test_live_openai_float_encoding_matches_base64( async def test_live_batch_embed_truncates_oversize_instead_of_dropping_batch( spec: LiveEmbeddingSpec, ) -> None: - """Regression for #569. Fails on main: simple_batch_embed raises - ValueError (or TypeError, before `on_oversize` existed) when any input - exceeds the per-item cap, so the rest of the batch is never embedded. - After the truncate path, one oversize item cannot drop the others. - """ + """on_oversize='truncate' keeps one vector per input when an item exceeds the cap.""" # Tiny cap so the oversize input stays cheap to tokenize and send. client = make_embedding_client(spec, max_input_tokens=32) oversize = " ".join(f"oversize-token-{index}" for index in range(200))