Skip to content

Fix bugs from comprehensive code review (v0.3.1) - #11

Merged
filthyrake merged 2 commits into
mainfrom
fix/code-review-bugs
Feb 9, 2026
Merged

filthyrake merged 2 commits into
mainfrom
fix/code-review-bugs

Conversation

@filthyrake

Copy link
Copy Markdown
Owner

Summary

Comprehensive bug fix release addressing findings from three independent code reviewers (Gafton, Margo, Scout). Fixes span 8 source files across reliability, correctness, and edge case handling.

HIGH severity fixes

  • SSE streaming: Add \n\n delimiters between events (were malformed — missing blank-line separators)
  • Streaming errors: Yield SSE error events on backend HTTP failures instead of silently truncating
  • vLLM CancelledError: Add missing cancellation handling to streaming path (was leaking connections)
  • Backend retry logic: Classify HTTP errors — 4xx = don't retry, 5xx/timeout/connect = retry (was breaking on first failure)
  • Graceful startup: Tolerate backend startup failures instead of crashing the app
  • temperature_override: Default changed from 0.0 to None so profiles without it passthrough client temperature

MEDIUM severity fixes

  • Streaming timeout: Per-chunk 60s timeout wrapper prevents stalled backends from hanging clients forever
  • SSE content field: Ensure content present in synthesized SSE delta (OpenAI client compat)
  • vLLM assert → RuntimeError: Won't be stripped by python -O
  • vLLM schema: arguments typed as string (not nested object) for constrained decoding
  • Retry budget: Minimum remaining buffer raised from 10s → 20s
  • Rescue regex: Handles quoted strings with parens; tries all bracket groups
  • Backend shutdown: 5s timeout prevents hanging during shutdown

LOW severity fixes

  • _think_re_cache: Bounded with lru_cache(maxsize=32) instead of unbounded dict
  • Escalation summary: Skip appending empty message when best response has valid calls
  • Validation: Filter additionalProperties errors from jsonschema (was double-reporting)
  • _in_flight counter: Encapsulated in _RequestCounter class with drain event

Version bump

  • 0.3.0 → 0.3.1

Test plan

  • All 77 existing tests pass
  • Deployed to 10.0.0.100 and tested against live Ollama backends
  • Proxy starts cleanly, health check passes, profiles loaded

🤖 Generated with Claude Code

filthyrake and others added 2 commits February 9, 2026 05:32
Addresses findings from Gafton, Margo, and Scout reviewers across 8 files:

- Fix SSE streaming: add \n\n delimiters (were dropping blank-line separators)
- Fix streaming errors: yield SSE error events instead of crashing silently
- Add CancelledError handling to vLLM streaming (was leaking connections)
- Replace vLLM assert with RuntimeError (safe under python -O)
- Fix vLLM constrained decoding schema (arguments as string, not object)
- Replace bare _in_flight counter with _RequestCounter class
- Graceful backend startup (tolerate failures, don't crash app)
- Add 5s timeout to backend shutdown (prevent hanging)
- Add per-chunk streaming timeout (60s, prevents stalled backend hangs)
- Ensure content field in synthesized SSE delta (OpenAI client compat)
- Classify HTTP errors in retry loop (4xx=permanent, 5xx/timeout=retry)
- Increase retry budget minimum buffer from 10s to 20s
- Fix pythonic rescue regex to handle quoted strings with parens
- Try all bracket groups in rescue (not just first match)
- Skip appending empty escalation summary message
- Bound _think_re_cache with lru_cache(maxsize=32)
- Fix temperature_override default (None=passthrough, not 0.0)
- Filter additionalProperties in jsonschema validation (was double-reporting)

All 77 tests pass.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@filthyrake
filthyrake merged commit 352a6c4 into main Feb 9, 2026
4 checks passed
@filthyrake
filthyrake deleted the fix/code-review-bugs branch February 9, 2026 13:40
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.

1 participant