fix: address review — anchor the shrink budget to the payload, not the ceiling

Six issues from review, one of which partly defeated the original fix.

1. The budget was seeded from `maxChars` rather than from the text being sent,
   so `truncate` (which only cuts when length > budget) made the first halvings
   byte-identical resends. A document below maxChars/2^4 — 1,875 chars at the
   default — could never shrink at all: five identical rejected calls and then
   the same permanent index hole this change exists to close, at 5x the cost.
   The budget now starts at min(maxChars, longest input).

2. The batch comment claimed one oversized member "cannot cost the short ones
   any content". Untrue: the budget is per-request, so a halving forced by one
   text also trims every other member above the new budget. The comment now
   says so, and seeds from the longest member. Also notes that a provider may
   reject on the request's aggregate token count, where trimming members is the
   right lever but the floor may arrive before the batch fits.

3. Intermediate attempts ran through `attempt(..., mayFailOpen=false)`, whose
   `handleFailure` logs before it rethrows — so every successful shrink emitted
   "Embedding failed" with a stack trace for a call that then succeeded, and a
   terminal failure logged twice with contradictory failOpen values. The retry
   loop is now `attemptOrThrow`, with logging left to the single terminal
   `handleFailure`.

4. The 4-arg `attempt` overload became unreachable once both callers moved to
   `embedWithShrink`; removed.

5. `givesUpAfterTheShrinkFloor...` asserted the attempt cap, not the floor —
   at a 30,000 ceiling the cap is always reached first. Renamed to say what it
   tests, and a real floor test added at a 1,500 ceiling.

6. The shrink log printed the budget as though it were the payload size, which
   would mislead exactly the person debugging (1).

New tests: shrinksRelativeToTheInputNotTheConfiguredCeiling (regression for 1,
fails on the previous commit) and stopsAtTheCharacterFloorRatherThanEmbedding
ATokenOfContent (coverage for 5).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: geekypunk

backend/src/main/java/com/dbaagent/service/EmbeddingService.java

@@ -118,7 +118,8 @@ public List<Double> createEmbedding(String text) {
         LlmEmbeddingProvider provider = registry.embeddingProvider(credentials.providerId());
         return embedWithShrink(provider, credentials,
                 budget -> provider.embed(truncate(text, budget), credentials),
-                List.of());
+                List.of(),
+                text == null ? 0 : text.length());
     }
 
     /**
@@ -140,25 +141,34 @@ public List<Double> createEmbedding(String text) {
      * about a rate limit or a bad credential.
      */
     private <T> T embedWithShrink(LlmEmbeddingProvider provider, LlmCredentials credentials,
-                                  java.util.function.IntFunction<T> call, T failOpenValue) {
-        int budget = maxChars;
+                                  java.util.function.IntFunction<T> call, T failOpenValue,
+                                  int longestInput) {
+        // Anchor to what is actually being sent, not to the configured ceiling. Seeding
+        // from maxChars makes the first halvings no-ops whenever the input is already
+        // under it — the same bytes, resent and rejected — and leaves anything below
+        // maxChars/2^MAX_SHRINK_ATTEMPTS unable to shrink at all, which is the very
+        // document this method exists to rescue.
+        int budget = Math.min(maxChars, Math.max(longestInput, 1));
         for (int shrink = 0; ; shrink++) {
             final int attemptBudget = budget;
             try {
-                // mayFailOpen=false: a CONTEXT_LENGTH rejection has to reach us. Fail-open
-                // would turn it into an empty vector indistinguishable from a real one.
-                return attempt(provider, credentials, () -> call.apply(attemptBudget), failOpenValue, false);
+                // attemptOrThrow, not attempt: a CONTEXT_LENGTH rejection has to reach us
+                // unlogged. Fail-open would turn it into an empty vector indistinguishable
+                // from a real one, and the logging path would report a failure for a call
+                // that is about to succeed.
+                return attemptOrThrow(provider, credentials, () -> call.apply(attemptBudget));
             } catch (RuntimeException e) {
                 LlmErrorCategory category = provider.classify(e);
                 if (category != LlmErrorCategory.CONTEXT_LENGTH
                         || shrink >= MAX_SHRINK_ATTEMPTS
                         || budget <= MIN_EMBEDDING_CHARS) {
                     // Out of room to shrink, or not a length problem: honour the operator's
-                    // fail-open setting exactly as before this method existed.
+                    // fail-open setting exactly as before this method existed. This is the
+                    // one place a terminal failure is logged.
                     return handleFailure(category, credentials, e, failOpenValue, failOpen);
                 }
                 int next = Math.max(MIN_EMBEDDING_CHARS, budget / 2);
-                log.warn("Embedding input rejected as too long at {} chars; retrying at {}", budget, next);
+                log.warn("Embedding rejected as too long at {} chars sent; retrying at {}", budget, next);
                 budget = next;
             }
         }
@@ -172,12 +182,23 @@ private <T> T embedWithShrink(LlmEmbeddingProvider provider, LlmCredentials cred
     public List<List<Double>> createEmbeddings(List<String> texts) {
         LlmCredentials credentials = requireCredentials();
         LlmEmbeddingProvider provider = registry.embeddingProvider(credentials.providerId());
-        // Shrinking the budget only touches texts longer than it, so one oversized member
-        // of a batch cannot cost the short ones any content.
+        // The budget is per-request, not per-member, so a halving forced by one oversized
+        // text also trims every other member above the new budget. That is a real cost and
+        // not something this method can avoid: the provider rejects the request, not a
+        // document, and it does not say which member was at fault. Seeding from the
+        // longest member keeps the first halving meaningful; callers that cannot afford
+        // collateral truncation should embed individually.
+        //
+        // Note also that a provider may reject on the request's AGGREGATE token count, in
+        // which case trimming members is the right lever but the floor may be reached
+        // before the batch fits.
+        int longest = texts.stream().filter(java.util.Objects::nonNull)
+                .mapToInt(String::length).max().orElse(0);
         return embedWithShrink(provider, credentials,
                 budget -> provider.embedBatch(
                         texts.stream().map(t -> truncate(t, budget)).toList(), credentials),
-                Collections.nCopies(texts.size(), List.of()));
+                Collections.nCopies(texts.size(), List.of()),
+                longest);
     }
 
     /**
@@ -230,10 +251,6 @@ public int dimensions() {
      * <p>The decision is {@link LlmErrorCategory#isRetryable()}, not a message substring:
      * that taxonomy exists precisely so retry policy stops being provider-specific.
      */
-    private <T> T attempt(LlmEmbeddingProvider provider, LlmCredentials credentials,
-                          Supplier<T> call, T failOpenValue) {
-        return attempt(provider, credentials, call, failOpenValue, failOpen);
-    }
 
     /**
      * As above, but with fail-open decided per call site rather than by configuration.
@@ -244,14 +261,33 @@ private <T> T attempt(LlmEmbeddingProvider provider, LlmCredentials credentials,
      */
     private <T> T attempt(LlmEmbeddingProvider provider, LlmCredentials credentials,
                           Supplier<T> call, T failOpenValue, boolean mayFailOpen) {
+        try {
+            return attemptOrThrow(provider, credentials, call);
+        } catch (RuntimeException e) {
+            return handleFailure(provider.classify(e), credentials, e, failOpenValue, mayFailOpen);
+        }
+    }
+
+    /**
+     * The retry loop with no opinion about failure: exhausted retries rethrow.
+     *
+     * <p>Separating this from {@link #handleFailure} is what lets {@link #embedWithShrink}
+     * treat a CONTEXT_LENGTH rejection as a step in a working algorithm rather than an
+     * incident. Routing intermediate attempts through the logging path made every
+     * successful shrink emit "Embedding failed" and a stack trace for a call that then
+     * succeeded — noise that would fire any alert keyed on that string, and log a terminal
+     * failure twice with contradictory failOpen values.
+     */
+    private <T> T attemptOrThrow(LlmEmbeddingProvider provider, LlmCredentials credentials,
+                                 Supplier<T> call) {
         for (int retries = 0; ; retries++) {
             try {
                 return call.get();
             } catch (RuntimeException e) {
                 LlmErrorCategory category = provider.classify(e);
                 if (retries >= MAX_RETRY_ATTEMPTS || !category.isRetryable()
                         || !backoff(retries, category, credentials, retryAfterHint(provider, e))) {
-                    return handleFailure(category, credentials, e, failOpenValue, mayFailOpen);
+                    throw e;
                 }
             }
         }

backend/src/test/java/com/dbaagent/service/EmbeddingServiceTest.java

@@ -509,7 +509,7 @@ void shrinksTheInputWhenTheProviderRejectsItAsTooLong() {
     }
 
     @Test
-    void givesUpAfterTheShrinkFloorAndStillHonoursFailOpen() {
+    void givesUpAfterTheAttemptCapAndStillHonoursFailOpen() {
         var resolver = mock(LlmConfigResolver.class);
         var registry = mock(LlmProviderRegistry.class);
         var provider = mock(LlmEmbeddingProvider.class);
@@ -522,7 +522,8 @@ void givesUpAfterTheShrinkFloorAndStillHonoursFailOpen() {
 
         var service = new EmbeddingService(resolver, registry, 30_000, true);
 
-        // Shrinking is bounded, so a pathological document cannot loop forever.
+        // Bounded by MAX_SHRINK_ATTEMPTS, not by MIN_EMBEDDING_CHARS: at a 30,000
+        // ceiling the cap is reached first. The floor is covered separately below.
         assertThat(service.createEmbedding("x".repeat(50_000))).isEmpty();
         verify(provider, times(5)).embed(anyString(), any()); // 30k, 15k, 7.5k, 3.75k, 1.875k
     }
@@ -545,4 +546,53 @@ void doesNotShrinkForFailuresThatShrinkingCannotFix() {
         // just multiply the failed calls.
         verify(provider, times(1)).embed(anyString(), any());
     }
+
+    @Test
+    void shrinksRelativeToTheInputNotTheConfiguredCeiling() {
+        var resolver = mock(LlmConfigResolver.class);
+        var registry = mock(LlmProviderRegistry.class);
+        var provider = mock(LlmEmbeddingProvider.class);
+
+        when(resolver.resolveEmbedding()).thenReturn(creds());
+        when(registry.embeddingProvider("openai")).thenReturn(provider);
+        when(provider.embed(anyString(), any())).thenAnswer(call -> {
+            String sent = call.getArgument(0);
+            if (sent.length() > 2_000) {
+                throw new RuntimeException("maximum context length is 8192 tokens");
+            }
+            return List.of(0.5);
+        });
+        when(provider.classify(any())).thenReturn(LlmErrorCategory.CONTEXT_LENGTH);
+
+        // 4,000 chars against a 30,000 ceiling. Seeding the budget from the ceiling would
+        // make the first halvings no-ops — 30,000 and 15,000 both send the same 4,000
+        // bytes — and burn the attempt cap on identical rejected calls.
+        new EmbeddingService(resolver, registry, 30_000, true)
+                .createEmbedding("x".repeat(4_000));
+
+        var captor = ArgumentCaptor.forClass(String.class);
+        verify(provider, times(2)).embed(captor.capture(), any());
+        assertThat(captor.getAllValues().get(0)).hasSize(4_000);
+        assertThat(captor.getAllValues().get(1)).hasSize(2_000);
+    }
+
+    @Test
+    void stopsAtTheCharacterFloorRatherThanEmbeddingATokenOfContent() {
+        var resolver = mock(LlmConfigResolver.class);
+        var registry = mock(LlmProviderRegistry.class);
+        var provider = mock(LlmEmbeddingProvider.class);
+
+        when(resolver.resolveEmbedding()).thenReturn(creds());
+        when(registry.embeddingProvider("openai")).thenReturn(provider);
+        when(provider.embed(anyString(), any()))
+                .thenThrow(new RuntimeException("maximum context length is 8192 tokens"));
+        when(provider.classify(any())).thenReturn(LlmErrorCategory.CONTEXT_LENGTH);
+
+        // A ceiling low enough that MIN_EMBEDDING_CHARS (1,000) is what stops the loop,
+        // not the attempt cap: 1,500 -> 1,000 -> give up.
+        var service = new EmbeddingService(resolver, registry, 1_500, true);
+
+        assertThat(service.createEmbedding("x".repeat(5_000))).isEmpty();
+        verify(provider, times(2)).embed(anyString(), any());
+    }
 }