Skip to content

fix(notes): harden LLM notes reliability - #585

Merged
goosewobbler merged 1 commit into
mainfrom
fix/llm-notes-reliability
Jul 20, 2026
Merged

goosewobbler merged 1 commit into
mainfrom
fix/llm-notes-reliability

Conversation

@goosewobbler

@goosewobbler goosewobbler commented Jul 19, 2026 •

Copy link
Copy Markdown
Owner

Summary

A batch of P2 LLM-notes reliability fixes (July 2026 direction review), all in packages/notes:

  • Independent per-task soft-fail (core/pipeline.ts): provider setup falls back to the raw context on failure; enhance+categorize, summarize, and releaseNotes each soft-fail in isolation, so a late failure keeps earlier (paid-for) results.
  • Anthropic thinking-model text path (llm/anthropic.ts): use content.find(b => b.type === 'text') instead of content[0], so a leading thinking block (Sonnet 5 / Fable 5) doesn't hard-fail non-schema tasks.
  • Temperature handling: Anthropic never forwards temperature (thinking models 400 on it — documented) and now reports honorsTemperature: false; the on-disk cache drops temperature from the key for such providers so toggling it no longer busts an identical response.
  • Retry classification (llm/retryable.ts, utils/retry.ts, all providers): fail fast on 4xx auth/validation; retry only timeout/429/5xx/network. LLMError carries a retryable flag and withRetry gained a shouldRetry predicate.
  • Chunk enhanceAndCategorize into ~30-entry batches with per-chunk corrective retry and per-chunk General fallback; merges same-named categories. ≤30 entries stays a single call.
  • Bound cross-package parallelism (utils/concurrency.ts): replace the unbounded Promise.all(contexts.map(...)) with a concurrency-bounded map.
  • Warn on partial enhancement (tasks/enhance.ts): surface a failure count when entries silently fall back to raw voice.
  • New AnthropicProvider unit spec: tool_use extraction, missing-tool-block error, thinking-before-text, timeout mapping, system-message splitting, temperature, and retry classification.

No Zod config schema changes, so no generated docs/schema updates.

Test plan

  • New/extended specs: test/unit/llm/anthropic.spec.ts, test/unit/llm/retryable.spec.ts, test/unit/utils/concurrency.spec.ts, test/integration/pipeline-softfail.spec.ts, plus additions to retry.spec.ts, cache.spec.ts, llm.spec.ts (chunking + isolated fallback).
  • Re-verified by the orchestrator the CI way: biome check . exit 0; forced turbo run typecheck test --force → 24/24.

Closes #543

🤖 Generated with Claude Code

https://claude.ai/code/session_01BT6fVBZrmExXZj7vQ29881

Greptile Summary

This PR hardens LLM notes reliability across packages/notes with a batch of complementary fixes: per-task soft-fail isolation in the pipeline, retry classification that fast-fails on 4xx auth/validation while retrying transient errors, chunked enhanceAndCategorize for large releases, bounded cross-package concurrency, and a thinking-model text-block fix for Anthropic. New unit and integration specs cover the key new paths.

  • Soft-fail refactor (pipeline.ts): provider setup and each LLM task now fail independently; a late summarize/releaseNotes failure keeps earlier enhancement and categorization results rather than discarding them.
  • Chunking (enhance-and-categorize.ts): enhanceAndCategorize splits large entry lists into ≤30-entry batches, each with its own corrective-retry budget and General-fallback isolation; mergeCategories reassembles same-named categories across chunks.
  • Retry classification (retryable.ts, retry.ts, all providers): withRetry gains a shouldRetry predicate; LLMError now carries a retryable flag; 4xx auth/validation errors fast-fail while 429/5xx/network/timeout errors retry.

Confidence Score: 5/5

Safe to merge. All changes are additive reliability improvements with no regressions to existing LLM task behaviour; the soft-fail paths are exercised by a dedicated integration test.

The three most complex changes — per-task soft-fail isolation in processWithLLM, chunked enhanceAndCategorize, and the mapWithConcurrency worker pool — are each internally consistent and covered by targeted unit/integration specs. The retry classification follows a clear, conservative 'default retryable, explicit fast-fail on 4xx' policy applied uniformly across all four providers. Error cause chaining and the temperature-cache fix address known gaps without touching unrelated code paths. No schema changes and no breaking public-interface alterations.

pipeline.ts carries the largest surface area (provider setup soft-fail + four independent task try/catch blocks + concurrency switch); worth a careful read-through, but the logic is straightforward and the integration test covers the key failure scenario.

Important Files Changed

Filename Overview
packages/notes/src/core/pipeline.ts Provider setup now soft-fails to raw context; all four LLM tasks are independently wrapped in try/catch; cross-package fan-out replaced with mapWithConcurrency. The let provider pattern (assigned in try, catch returns early) is correctly handled by TypeScript's CFA. Largest surface area of the change and the most complex; logic is correct.
packages/notes/src/llm/tasks/enhance-and-categorize.ts Chunking logic extracted into enhanceAndCategorizeChunk; mergeCategories merges same-named categories across chunks; system prompt is built once and shared across all chunks. Sequential processing is intentional for rate-limit safety. Category merge is case-sensitive (consistent with LLM prompt constraints).
packages/notes/src/llm/retryable.ts New file with three well-scoped predicates: isRetryableStatus (4xx fast-fail / 5xx+429/408/409/425 retry), isRetryableProviderError (delegates to status then code, defaults retryable for unknowns), and isRetryableLLMError (reads LLMError.retryable, defaults true for unclassified). Logic is sound and consistently applied across providers.
packages/notes/src/utils/concurrency.ts New bounded-concurrency map utility using a shared next counter and failed flag behind a worker-pool pattern. Order is preserved via index-based slot assignment. JS single-thread guarantees no race on next/failed. Edge cases (empty input, limit <= 0, NaN limit) all handled. Worker-pool stops scheduling new items after first failure; in-flight calls still settle.
packages/notes/src/utils/retry.ts shouldRetry predicate added cleanly; checked before the sleep decision so non-retryable errors break out of the loop immediately without burning any delay budget. Correct across all attempt counts including the last.
packages/notes/src/llm/anthropic.ts content.find(b => b.type === 'text') correctly handles thinking-model responses where content[0] would be a thinking block. honorsTemperature: false set to prevent cache busts and 400s from thinking models. Error classification delegates to isRetryableProviderError with cause chaining.
packages/notes/src/errors/index.ts LLMError gains a retryable?: boolean flag and a constructor that accepts {retryable, cause}. Cause is chained via direct property assignment (valid in ES2022+ environments). Addresses the previous review thread's request for cause chaining.
packages/notes/src/llm/cache.ts Cache key omits temperature when honorsTemperature === false (Anthropic), preventing cache misses when the ignored temperature option changes between runs. Clean conditional keying.
packages/notes/src/llm/ollama.ts 401/403 hardcoded to retryable: false with helpful key-hint message; other non-OK statuses use isRetryableStatus. honorsTemperature: true added. Error wrapping gains cause and retryable classification consistently with other providers.
packages/notes/src/llm/openai.ts honorsTemperature: true and isRetryableProviderError classification added in line with other providers. Cause chained on non-timeout errors.
packages/notes/src/llm/openai-compatible.ts Same honorsTemperature and isRetryableProviderError treatment as openai.ts. Consistent with the pattern across all providers.
packages/notes/src/llm/provider.ts honorsTemperature?: boolean added to ProviderCapabilities with clear JSDoc. Optional (absent = honored) so existing providers without the field default to the common case without a migration.
packages/notes/src/llm/tasks/enhance.ts failures counter added; warn emitted when any per-entry soft-fail occurs. Surfaces mixed-voice output that would previously be silent. Minimal and correct change.
packages/notes/src/llm/defaults.ts enhanceCategorizeChunkSize: 30 added to LLM_DEFAULTS with explanatory comment. Not exposed in the Zod config schema yet (intentional per PR description).
packages/notes/test/integration/pipeline-softfail.spec.ts Integration test validates the key soft-fail guarantee: enhance+categorize output survives a later summarize failure. Mocks the OpenAI completions path with response_format discrimination. Correctly sets maxAttempts: 1 to skip retry noise.
packages/notes/test/unit/llm/anthropic.spec.ts New spec covers tool_use extraction, missing-tool-block error, thinking-before-text ordering, plain text response, no-text-block error, system message splitting, temperature non-forwarding, timeout mapping, and retry classification. Good coverage of the changed behaviour.
packages/notes/test/unit/llm/retryable.spec.ts Unit tests for isRetryableStatus, isRetryableProviderError, and isRetryableLLMError covering the key decision boundaries (401 vs 429, network codes, unknown shapes, LLMError.retryable flag).
packages/notes/test/unit/utils/concurrency.spec.ts Tests cover order preservation, peak concurrency enforcement, empty input, non-positive limit normalisation, and early-stop on failure. Complete for the implemented contract.
packages/notes/test/unit/llm.spec.ts Two new chunking specs added: one for happy-path 35-entry split (2 calls, merged General category) and one for isolated chunk failure (first 30 succeed as New, tail 5 fall back to General). Both correctly count calls and validate the merged result shape.
packages/notes/test/unit/llm/cache.spec.ts New test confirms that toggling temperature does not bust the cache for a provider with honorsTemperature: false (mirrors Anthropic behaviour). Correctly asserts provider.calls === 1 after two requests with different temperatures.
packages/notes/test/unit/retry.spec.ts Two new tests for shouldRetry: one asserting immediate stop (calls === 1) when predicate returns false, one asserting full retry budget is consumed (calls === 3) when predicate returns true.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[runPipeline] --> B[mapWithConcurrency\nbounded by concurrency knob]
    B --> C[processWithLLM per package]
    C --> D{Provider setup}
    D -- throws --> E[warn + return raw context\nsoft-fail]
    D -- success --> F{tasks.enhance\n&& tasks.categorize?}
    F -- yes --> G[enhanceAndCategorize\nchunked into <= 30 entries]
    G --> H[enhanceAndCategorizeChunk\nper chunk]
    H --> I{runCorrectiveTask\nsucceeds?}
    I -- yes --> J[merge categories\npreserve order]
    I -- no LLMError --> K[General fallback\nisolated to chunk]
    J --> L[tasks.summarize?]
    K --> L
    F -- no --> L
    L -- yes --> M[summarizeEntries\ntry/catch soft-fail]
    L -- no --> N[tasks.releaseNotes?]
    M -- fails --> O[warn + keep prior results]
    M -- success --> N
    O --> N
    N -- yes --> P[generateReleaseNotes\ntry/catch soft-fail]
    N -- no --> Q[return enhanced context]
    P --> Q
    P -- fails --> Q

    subgraph withRetry
        R[attempt fn] -- throws --> S{shouldRetry isRetryableLLMError?}
        S -- false 4xx --> T[fast-fail immediately]
        S -- true 429/5xx/network --> U[sleep + retry]
        U --> R
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[runPipeline] --> B[mapWithConcurrency\nbounded by concurrency knob]
    B --> C[processWithLLM per package]
    C --> D{Provider setup}
    D -- throws --> E[warn + return raw context\nsoft-fail]
    D -- success --> F{tasks.enhance\n&& tasks.categorize?}
    F -- yes --> G[enhanceAndCategorize\nchunked into <= 30 entries]
    G --> H[enhanceAndCategorizeChunk\nper chunk]
    H --> I{runCorrectiveTask\nsucceeds?}
    I -- yes --> J[merge categories\npreserve order]
    I -- no LLMError --> K[General fallback\nisolated to chunk]
    J --> L[tasks.summarize?]
    K --> L
    F -- no --> L
    L -- yes --> M[summarizeEntries\ntry/catch soft-fail]
    L -- no --> N[tasks.releaseNotes?]
    M -- fails --> O[warn + keep prior results]
    M -- success --> N
    O --> N
    N -- yes --> P[generateReleaseNotes\ntry/catch soft-fail]
    N -- no --> Q[return enhanced context]
    P --> Q
    P -- fails --> Q

    subgraph withRetry
        R[attempt fn] -- throws --> S{shouldRetry isRetryableLLMError?}
        S -- false 4xx --> T[fast-fail immediately]
        S -- true 429/5xx/network --> U[sleep + retry]
        U --> R
    end
Loading

Reviews (3): Last reviewed commit: "fix(notes): harden LLM notes reliability..." | Re-trigger Greptile

@goosewobbler goosewobbler added this to the 0.42.0 milestone Jul 19, 2026
@github-actions

github-actions Bot commented Jul 19, 2026 •

Copy link
Copy Markdown
Contributor

Standing release PR: #582 · v0.41.1 queued (4 packages) · open 2h 49m · ✅ ready to merge

Release Preview — this PR contributes no changes

No bump label detected.
Note: Add bump:patch, bump:minor, or bump:major to trigger a release.

Currently queued for release

Package Current Next
monorepo 0.41.0 0.41.1

Updated automatically by ReleaseKit

Comment thread packages/notes/src/llm/retryable.ts Outdated
Comment thread packages/notes/src/errors/index.ts Outdated
@goosewobbler
goosewobbler force-pushed the fix/llm-notes-reliability branch 3 times, most recently from 1f385a9 to 8020502 Compare July 19, 2026 23:56
@goosewobbler goosewobbler changed the title fix(notes): harden LLM notes reliability (soft-fail, retry, chunking) fix(notes): harden LLM notes reliability Jul 19, 2026
Independently soft-fail each LLM task so a late summarize/releaseNotes
failure no longer discards already-computed enhancement; find the
Anthropic text block instead of reading content[0] for thinking-model
forward-compat; classify retryable vs non-retryable errors so 4xx
auth/validation fails fast while timeout/429/5xx/network retry; chunk
enhanceAndCategorize into bounded batches with per-chunk corrective
retry; bound cross-package fan-out with the existing concurrency config;
exclude temperature from the Anthropic cache identity (it is never
forwarded); and warn on partial per-entry enhancement. Adds the missing
AnthropicProvider unit spec.

Closes #543

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BT6fVBZrmExXZj7vQ29881
@goosewobbler
goosewobbler force-pushed the fix/llm-notes-reliability branch from 8020502 to 6be1d43 Compare July 20, 2026 00:12
@goosewobbler
goosewobbler merged commit ba06b18 into main Jul 20, 2026
26 of 28 checks passed
@goosewobbler
goosewobbler deleted the fix/llm-notes-reliability branch July 20, 2026 07:49
@github-actions github-actions Bot mentioned this pull request Jul 20, 2026
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.

LLM notes reliability batch: per-task soft-fail, thinking-model text path, retry classification, chunking, concurrency bound

1 participant