-
Notifications
You must be signed in to change notification settings - Fork 0
fix: address all review comments — rsync safety, defensive JSON parsing, type hygiene #265
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
fb94aec
test: add canonical endpoint verification script
3201825
fix: LiteLLM UI credentials and Langfuse NEXTAUTH_URL
73b99fe
test: add LiteLLM UI and Langfuse web UI to canonical endpoint tests
664a1cd
test: add visualizer, infra health, and LiteLLM direct chat to verifi…
678a8cc
test: add canonical POST chat completion to verification
208d2a3
fix: defensive choices access + differentiate skipped from passed
c591995
docs: add canonical endpoint verification to README
993b195
fix: address review comments — refactor chat parsing, env-aware infra…
f1d9cea
fix: defensive chat response parsing in all classifier scripts
477d5e8
fix: address review comments on PR #259
585d8ac
fix: address review comments on PR #260
86edd69
fix: correct health check endpoints for LiteLLM and Langfuse
2dde176
feat: add upgrade-prod.sh for release-driven prod sync
2286f6e
docs: update health check table with correct endpoints
010d8a9
fix: address all PR #261 review comments
ad2ec89
fix: use try/except for intra-package imports in verification scripts
6a27eac
fix: address bot review comments — rsync safety, non-interactive guar…
ac8690a
fix: guard .json() calls with status check, use or fallback for .env …
941fdd3
fix: address Gemini Code Assist review comments
66abaa0
Update README.md
sheepdestroyer 561ae39
fix: address Sourcery, Gemini, CodeRabbit review comments
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| # LLM-Routing scripts package |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| """Shared helpers for defensive chat completion response parsing. | ||
|
|
||
| Used by the canonical endpoint verification script and the classifier scripts | ||
| to safely extract content and reasoning_content from OpenAI-compatible API responses. | ||
| """ | ||
|
|
||
| from typing import Any | ||
|
|
||
|
|
||
| def _normalize_chat_content(value: Any) -> str: | ||
| """Normalize structured content payloads into a plain string. | ||
|
|
||
| Handles plain strings, lists of strings, lists of dicts with | ||
| ``text`` / ``content`` keys, and dicts with ``text`` / ``content`` keys. | ||
| Returns an empty string for None or unrecognised shapes. | ||
| """ | ||
| if value is None: | ||
| return "" | ||
| if isinstance(value, str): | ||
| return value.strip() | ||
| if isinstance(value, list): | ||
| parts: list[str] = [] | ||
| for item in value: | ||
| if isinstance(item, str): | ||
| parts.append(item) | ||
| elif isinstance(item, dict): | ||
| text = item.get("text") | ||
| if isinstance(text, str): | ||
| parts.append(text) | ||
| elif "content" in item: | ||
| nested = _normalize_chat_content(item.get("content")) | ||
| if nested: | ||
| parts.append(nested) | ||
| return "".join(parts) | ||
| if isinstance(value, dict): | ||
| text = value.get("text") | ||
| if isinstance(text, str): | ||
| return text | ||
| if "content" in value: | ||
| return _normalize_chat_content(value.get("content")) | ||
| return "" | ||
|
|
||
|
|
||
| def parse_chat_response(data: Any) -> tuple[str, str]: | ||
| """Safely extract content and reasoning_content from a chat completion response. | ||
|
|
||
| Args: | ||
| data: Parsed JSON response from a chat completion endpoint (expected to be a dict). | ||
|
|
||
| Returns: | ||
| (content, reasoning_content) — both may be empty strings. | ||
| """ | ||
| if not isinstance(data, dict): | ||
| return "", "" | ||
| choices = data.get("choices") | ||
| if not isinstance(choices, list) or not choices: | ||
| return "", "" | ||
| first_choice = choices[0] | ||
| if not isinstance(first_choice, dict): | ||
| return "", "" | ||
| message = first_choice.get("message") | ||
| if not isinstance(message, dict): | ||
| return "", "" | ||
| content = _normalize_chat_content(message.get("content")) | ||
| reasoning = _normalize_chat_content(message.get("reasoning_content")) | ||
| return content, reasoning | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (bug_risk): Handle non-string
content/reasoning_contentpayloads (e.g., list-based content) defensively.The parser currently assumes
message['content']/message['reasoning_content']are plain strings. Some OpenAI-compatible providers return structured payloads (e.g., lists of{type: 'text', text: ...}), so theisinstance(..., str)check drops valid content and produces downstream "ERROR" values. Please normalize common structured shapes (e.g., list-of-dicts) into text before defaulting to an empty string, so this stays robust across providers.Suggested implementation: