open-api-evals-script #22
Conversation
|
""" WalkthroughTwo new Python scripts have been added: Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant generate_openapi.py
participant evaluators.generated.ts
participant openapi.json
participant generate_mdx_list.py
participant evaluators-list.mdx
User->>generate_openapi.py: Run script
generate_openapi.py->>evaluators.generated.ts: Read TypeScript file
generate_openapi.py->>generate_openapi.py: Parse evaluators and settings
generate_openapi.py->>generate_openapi.py: Generate OpenAPI schema with components and paths
generate_openapi.py->>openapi.json: Write OpenAPI JSON schema
generate_openapi.py->>User: Print progress/errors
User->>generate_mdx_list.py: Run script
generate_mdx_list.py->>openapi.json: Read OpenAPI JSON
generate_mdx_list.py->>generate_mdx_list.py: Categorize evaluators
generate_mdx_list.py->>generate_mdx_list.py: Generate MDX content
generate_mdx_list.py->>evaluators-list.mdx: Write MDX file
generate_mdx_list.py->>User: Print summary/errors
Estimated code review effort3 (30–60 minutes) Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
scripts/generate_openapi.py (1)
123-124: Complex regex pattern may be difficult to maintain.The regex pattern for parsing settings is complex and tries to handle nested structures. Consider using a more robust parsing approach or documenting the expected format clearly.
For better maintainability, consider:
- Using a proper TypeScript parser library if available
- Adding comprehensive tests for edge cases
- Documenting the expected TypeScript format this regex handles
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/generate_openapi.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
scripts/generate_openapi.py
10-10: typing.List imported but unused
Remove unused import
(F401)
10-10: typing.Optional imported but unused
Remove unused import
(F401)
30-30: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
🔇 Additional comments (2)
scripts/generate_openapi.py (2)
35-46: Consider edge cases in brace counting logic.The current implementation counts braces without considering that braces might appear within strings, which could lead to incorrect parsing.
For example, if an evaluator contains a string like
description: "Use { and } in text", the brace counting might fail. Consider using a more robust parsing approach or adding string detection to skip braces within quotes.
589-621: Well-structured entry point with proper error handling.The main function correctly handles file paths, errors, and provides informative output messages.
scripts/generate_openapi.py
Outdated
| "required": True, | ||
| }, | ||
| "responses": { | ||
| "200": { | ||
| "description": "Successful evaluation", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "array", | ||
| "items": { | ||
| "$ref": "#/components/schemas/EvaluationResult" | ||
| }, | ||
| } | ||
| } | ||
| }, | ||
| }, | ||
| "400": { | ||
| "description": "Bad request", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "object", | ||
| "properties": {"detail": {"type": "string"}}, | ||
| } | ||
| } | ||
| }, | ||
| }, | ||
| "500": { | ||
| "description": "Internal server error", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "object", | ||
| "properties": {"detail": {"type": "string"}}, | ||
| } | ||
| } | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| } | ||
| else: | ||
| # Create the path without settings for evaluators with no settings | ||
| schema["paths"][path_key] = { | ||
| "post": { | ||
| "summary": evaluator["name"], | ||
| "description": evaluator["description"], | ||
| "operationId": f"{evaluator_id.replace('.', '_').replace('/', '_')}_evaluate", | ||
| "requestBody": { | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "$ref": f"#/components/schemas/{request_schema_name}" | ||
| } | ||
| } | ||
| }, | ||
| "required": True, | ||
| }, | ||
| "responses": { | ||
| "200": { | ||
| "description": "Successful evaluation", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "array", | ||
| "items": { | ||
| "$ref": "#/components/schemas/EvaluationResult" | ||
| }, | ||
| } | ||
| } | ||
| }, | ||
| }, | ||
| "400": { | ||
| "description": "Bad request", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "object", | ||
| "properties": {"detail": {"type": "string"}}, | ||
| } | ||
| } | ||
| }, | ||
| }, | ||
| "500": { | ||
| "description": "Internal server error", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "object", | ||
| "properties": {"detail": {"type": "string"}}, | ||
| } | ||
| } | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| return schema | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Break down this large function for better maintainability.
This function spans over 400 lines and handles multiple responsibilities. Consider extracting helper functions for:
- Creating component schemas
- Generating evaluator-specific schemas
- Creating path definitions
🤖 Prompt for AI Agents
In scripts/generate_openapi.py from lines 176 to 587, the
generate_openapi_schema function is very large and handles multiple tasks,
making it hard to maintain. Refactor by extracting helper functions for creating
component schemas, generating evaluator-specific schemas, and creating path
definitions. This will modularize the code, improve readability, and simplify
future updates.
scripts/generate_openapi.py
Outdated
| } | ||
| } | ||
| }, | ||
| "required": True, | ||
| }, | ||
| "responses": { | ||
| "200": { | ||
| "description": "Successful evaluation", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "array", | ||
| "items": { | ||
| "$ref": "#/components/schemas/EvaluationResult" | ||
| }, | ||
| } | ||
| } | ||
| }, | ||
| }, | ||
| "400": { | ||
| "description": "Bad request", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "object", | ||
| "properties": {"detail": {"type": "string"}}, | ||
| } | ||
| } | ||
| }, | ||
| }, | ||
| "500": { | ||
| "description": "Internal server error", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "object", | ||
| "properties": {"detail": {"type": "string"}}, | ||
| } | ||
| } | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| } | ||
| else: | ||
| # Create the path without settings for evaluators with no settings | ||
| schema["paths"][path_key] = { | ||
| "post": { | ||
| "summary": evaluator["name"], | ||
| "description": evaluator["description"], | ||
| "operationId": f"{evaluator_id.replace('.', '_').replace('/', '_')}_evaluate", | ||
| "requestBody": { | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "$ref": f"#/components/schemas/{request_schema_name}" | ||
| } | ||
| } | ||
| }, | ||
| "required": True, | ||
| }, | ||
| "responses": { | ||
| "200": { | ||
| "description": "Successful evaluation", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "array", | ||
| "items": { | ||
| "$ref": "#/components/schemas/EvaluationResult" | ||
| }, | ||
| } | ||
| } | ||
| }, | ||
| }, | ||
| "400": { | ||
| "description": "Bad request", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "object", | ||
| "properties": {"detail": {"type": "string"}}, | ||
| } | ||
| } | ||
| }, | ||
| }, | ||
| "500": { | ||
| "description": "Internal server error", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "type": "object", | ||
| "properties": {"detail": {"type": "string"}}, | ||
| } | ||
| } | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Eliminate code duplication in path generation.
The code for creating paths with settings (lines 463-528) and without settings (lines 531-584) is nearly identical, differing only in the request body schema.
Extract the common path generation logic into a helper function:
def create_evaluator_path(evaluator_id, evaluator, request_schema_name, has_settings=False, settings_schema_name=None):
"""Create path definition for an evaluator."""
path_def = {
"post": {
"summary": evaluator["name"],
"description": evaluator["description"],
"operationId": f"{evaluator_id.replace('.', '_').replace('/', '_')}_evaluate",
"requestBody": {
"content": {
"application/json": {
"schema": {"$ref": f"#/components/schemas/{request_schema_name}"}
}
},
"required": True,
},
"responses": {
"200": {
"description": "Successful evaluation",
"content": {
"application/json": {
"schema": {
"type": "array",
"items": {"$ref": "#/components/schemas/EvaluationResult"},
}
}
},
},
"400": {
"description": "Bad request",
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {"detail": {"type": "string"}},
}
}
},
},
"500": {
"description": "Internal server error",
"content": {
"application/json": {
"schema": {
"type": "object",
"properties": {"detail": {"type": "string"}},
}
}
},
},
},
}
}
# Modify schema for settings if needed
if has_settings and settings_schema_name:
path_def["post"]["requestBody"]["content"]["application/json"]["schema"] = {
"allOf": [
{"$ref": f"#/components/schemas/{request_schema_name}"},
{
"type": "object",
"properties": {
"settings": {"$ref": f"#/components/schemas/{settings_schema_name}"}
},
},
]
}
return path_defThen replace the duplicated code blocks with:
if settings_schema["properties"]:
# ... existing settings schema creation code ...
schema["paths"][path_key] = create_evaluator_path(
evaluator_id, evaluator, request_schema_name,
has_settings=True, settings_schema_name=settings_schema_name
)
else:
schema["paths"][path_key] = create_evaluator_path(
evaluator_id, evaluator, request_schema_name
)🤖 Prompt for AI Agents
In scripts/generate_openapi.py around lines 463 to 584, the code for generating
evaluator paths with and without settings is duplicated except for the request
body schema. To fix this, extract the common path generation logic into a helper
function that accepts parameters for evaluator details and whether settings are
included. This function should build the path definition and conditionally
modify the request body schema if settings are present. Then replace the
duplicated blocks by calling this helper with appropriate arguments based on the
presence of settings.
| matches = list(re.finditer(evaluator_pattern, content)) | ||
| print(f"Found {len(matches)} evaluator matches") | ||
|
|
||
| for i, match in enumerate(matches): |
There was a problem hiding this comment.
Rename unused loop variable i to _i.
The loop control variable is not used within the loop body.
-for i, match in enumerate(matches):
+for _i, match in enumerate(matches):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for i, match in enumerate(matches): | |
| for _i, match in enumerate(matches): |
🧰 Tools
🪛 Ruff (0.12.2)
30-30: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
🤖 Prompt for AI Agents
In scripts/generate_openapi.py at line 30, the loop variable `i` is declared but
not used inside the loop. Rename `i` to `_i` to indicate that this variable is
intentionally unused and to follow Python conventions for unused variables.
| import re | ||
| import sys | ||
| from pathlib import Path | ||
| from typing import Dict, Any, List, Optional |
There was a problem hiding this comment.
Remove unused imports List and Optional.
These type hints are imported but never used in the code.
-from typing import Dict, Any, List, Optional
+from typing import Dict, Any📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from typing import Dict, Any, List, Optional | |
| from typing import Dict, Any |
🧰 Tools
🪛 Ruff (0.12.2)
10-10: typing.List imported but unused
Remove unused import
(F401)
10-10: typing.Optional imported but unused
Remove unused import
(F401)
🤖 Prompt for AI Agents
In scripts/generate_openapi.py at line 10, the imports List and Optional from
typing are not used anywhere in the code. Remove List and Optional from the
import statement to keep the imports clean and avoid unnecessary dependencies.
| elif value_str.replace(".", "").isdigit(): | ||
| return float(value_str) |
There was a problem hiding this comment.
Fix incorrect float detection logic.
The current implementation would incorrectly identify strings like "1.2.3" as floats.
-elif value_str.replace(".", "").isdigit():
- return float(value_str)
+else:
+ try:
+ return float(value_str)
+ except ValueError:
+ pass📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| elif value_str.replace(".", "").isdigit(): | |
| return float(value_str) | |
| else: | |
| try: | |
| return float(value_str) | |
| except ValueError: | |
| pass |
🤖 Prompt for AI Agents
In scripts/generate_openapi.py around lines 164 to 165, the current float
detection logic incorrectly treats strings like "1.2.3" as valid floats. Modify
the condition to properly validate the float format, ensuring the string
contains only one decimal point and all other characters are digits. This can be
done by checking that the string splits into exactly two parts by a single dot
and both parts are digits before converting to float.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
scripts/generate_openapi.py (5)
10-10: Remove unused imports.The
ListandOptionaltype hints are imported but never used in the code.-from typing import Dict, Any, List, Optional +from typing import Dict, Any
56-56: Rename unused loop variable.The loop control variable
iis not used within the loop body.-for i, match in enumerate(matches): +for _i, match in enumerate(matches):
282-283: Fix incorrect float detection logic.The current implementation incorrectly identifies strings like "1.2.3" as valid floats.
-elif value_str.replace(".", "").isdigit(): - return float(value_str) +else: + try: + return float(value_str) + except ValueError: + pass
300-733: Break down this large function for better maintainability.This function spans over 400 lines and handles multiple responsibilities. Consider extracting helper functions for creating component schemas, generating evaluator-specific schemas, and creating path definitions.
610-731: Eliminate code duplication in path generation.The code for creating paths with settings and without settings is nearly identical, differing only in the request body schema. Extract the common path generation logic into a helper function.
🧹 Nitpick comments (3)
scripts/generate_openapi.py (3)
247-247: Rename unused loop variable.The loop control variable
iis not used within the loop body.-for i, char in enumerate(content[bracket_start:], bracket_start): +for _i, char in enumerate(content[bracket_start:], bracket_start):
762-764: Improve error handling specificity.The broad exception handling could mask different types of errors. Consider handling specific exceptions differently.
- except Exception as error: - print(f"Error generating OpenAPI schema: {error}") - sys.exit(1) + except FileNotFoundError as error: + print(f"File not found: {error}") + sys.exit(1) + except (json.JSONDecodeError, ValueError) as error: + print(f"Data parsing error: {error}") + sys.exit(1) + except Exception as error: + print(f"Unexpected error generating OpenAPI schema: {error}") + sys.exit(1)
1-11: Consider adding type hints to improve code maintainability.The script would benefit from more comprehensive type annotations, especially for complex return types and function parameters.
#!/usr/bin/env python3 """ Python script to generate OpenAPI JSON from evaluators.generated.ts """ import json import re import sys from pathlib import Path +from typing import Dict, Any, Union
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/generate_openapi.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
scripts/generate_openapi.py
10-10: typing.List imported but unused
Remove unused import
(F401)
10-10: typing.Optional imported but unused
Remove unused import
(F401)
56-56: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
247-247: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
288-288: Do not use bare except
(E722)
294-294: Do not use bare except
(E722)
🔇 Additional comments (1)
scripts/generate_openapi.py (1)
736-768: The main function orchestrates the process well.Good error handling, clear progress output, and proper file path resolution. The function correctly handles the overall workflow.
| # Array value - try to parse as JSON | ||
| try: | ||
| return json.loads(value_str) | ||
| except: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace bare except clauses with specific exception handling.
Bare except statements can mask unexpected errors and make debugging difficult.
- except:
+ except (json.JSONDecodeError, ValueError):
return []- except:
+ except (json.JSONDecodeError, ValueError):
return {}Also applies to: 294-294
🧰 Tools
🪛 Ruff (0.12.2)
288-288: Do not use bare except
(E722)
🤖 Prompt for AI Agents
In scripts/generate_openapi.py at lines 288 and 294, replace the bare except
clauses with specific exception types to catch only expected errors. Identify
the exceptions that the try block might raise and handle those explicitly
instead of using a generic except. This improves error handling clarity and
debugging.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
scripts/generate_openapi.py (7)
10-10: Remove unused importsListandOptional.These type hints are imported but never used in the code.
-from typing import Dict, Any, List, Optional +from typing import Dict, Any
56-56: Rename unused loop variableito_i.The loop control variable is not used within the loop body.
-for i, match in enumerate(matches): +for _i, match in enumerate(matches):
274-274: Rename unused loop variableito_i.The loop control variable is not used within the loop body.
-for i, char in enumerate(content[bracket_start:], bracket_start): +for _i, char in enumerate(content[bracket_start:], bracket_start):
322-323: Fix incorrect float detection logic.The current implementation would incorrectly identify strings like "1.2.3" as floats.
-elif value_str.replace(".", "").isdigit(): - return float(value_str) +else: + try: + return float(value_str) + except ValueError: + pass
328-328: Replace bare except clauses with specific exception handling.Bare
exceptstatements can mask unexpected errors and make debugging difficult.- except: + except (json.JSONDecodeError, ValueError): return []- except: + except (json.JSONDecodeError, ValueError): return {}Also applies to: 333-333, 339-339, 344-344
350-783: Break down this large function for better maintainability.This function spans over 400 lines and handles multiple responsibilities. Consider extracting helper functions for:
- Creating component schemas
- Generating evaluator-specific schemas
- Creating path definitions
660-781: Eliminate code duplication in path generation.The code for creating paths with settings (lines 660-725) and without settings (lines 728-781) is nearly identical, differing only in the request body schema. Extract the common path generation logic into a helper function.
🧹 Nitpick comments (1)
scripts/generate_openapi.py (1)
232-307: Remove extensive debug output or make it conditional.The code contains numerous debug print statements that clutter the output and may impact performance in production.
Consider adding a debug flag or using a proper logging framework:
import logging # Configure logging logging.basicConfig(level=logging.INFO) logger = logging.getLogger(__name__) # Replace debug prints with: if "competitors" in content: logger.debug(f"extract_default_value called with content: {content}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/generate_openapi.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
scripts/generate_openapi.py
10-10: typing.List imported but unused
Remove unused import
(F401)
10-10: typing.Optional imported but unused
Remove unused import
(F401)
56-56: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
274-274: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
328-328: Do not use bare except
(E722)
333-333: Do not use bare except
(E722)
339-339: Do not use bare except
(E722)
344-344: Do not use bare except
(E722)
🔇 Additional comments (1)
scripts/generate_openapi.py (1)
786-818: LGTM! Well-structured main function with proper error handling.The main function is well-organized with clear error handling and informative output messages.
| for j, char in enumerate(content[start_pos:], start_pos): | ||
| if char == "{": | ||
| brace_count += 1 | ||
| elif char == "}": | ||
| brace_count -= 1 | ||
| if brace_count == 0: | ||
| end_pos = j + 1 | ||
| break |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract repeated brace matching logic into a helper function.
The brace matching logic is duplicated across multiple functions, making the code harder to maintain.
def find_matching_brace(content: str, start_pos: int, open_char: str = '{') -> int:
"""Find the position of the matching closing brace."""
close_char = '}' if open_char == '{' else ']'
brace_count = 0
end_pos = start_pos
for i, char in enumerate(content[start_pos:], start_pos):
if char == open_char:
brace_count += 1
elif char == close_char:
brace_count -= 1
if brace_count == 0:
end_pos = i + 1
break
return end_posThen replace the duplicated logic with calls to this helper function.
Also applies to: 64-71, 147-154, 182-189, 274-281
🤖 Prompt for AI Agents
In scripts/generate_openapi.py around lines 37 to 44 (and similarly at 64-71,
147-154, 182-189, 274-281), the brace matching logic is duplicated. Extract this
logic into a helper function named find_matching_brace that takes content,
start_pos, and optionally open_char, and returns the position of the matching
closing brace. Replace all duplicated brace matching code blocks with calls to
this helper function to improve maintainability and reduce redundancy.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (10)
scripts/generate_openapi.py (10)
10-10: Remove unused imports.The imports
ListandOptionalfrom typing are not used anywhere in the code.
56-56: Rename unused loop variable.The loop control variable
iis not used within the loop body.
270-271: Fix incorrect float detection logic.The current implementation would incorrectly identify strings like "1.2.3" as floats.
276-276: Replace bare except clauses with specific exception handling.Bare
exceptstatements can mask unexpected errors and make debugging difficult.
281-281: Replace bare except clauses with specific exception handling.Bare
exceptstatements can mask unexpected errors and make debugging difficult.
287-287: Replace bare except clauses with specific exception handling.Bare
exceptstatements can mask unexpected errors and make debugging difficult.
292-292: Replace bare except clauses with specific exception handling.Bare
exceptstatements can mask unexpected errors and make debugging difficult.
298-731: Break down this large function for better maintainability.This function spans over 400 lines and handles multiple responsibilities.
37-44: Extract repeated brace matching logic into a helper function.The brace matching logic is duplicated across multiple functions.
608-729: Eliminate code duplication in path generation.The code for creating paths with settings and without settings is nearly identical.
🧹 Nitpick comments (1)
scripts/generate_openapi.py (1)
246-246: Rename unused loop variableito_i.The loop control variable
iis declared but not used inside the loop body. Following Python conventions, unused variables should be prefixed with underscore.- for i, char in enumerate(content[bracket_start:], bracket_start): + for _i, char in enumerate(content[bracket_start:], bracket_start):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/generate_openapi.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
scripts/generate_openapi.py
10-10: typing.List imported but unused
Remove unused import
(F401)
10-10: typing.Optional imported but unused
Remove unused import
(F401)
56-56: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
246-246: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
276-276: Do not use bare except
(E722)
281-281: Do not use bare except
(E722)
287-287: Do not use bare except
(E722)
292-292: Do not use bare except
(E722)
🔇 Additional comments (1)
scripts/generate_openapi.py (1)
734-766: Well-structured main function with proper error handling.The main function demonstrates good practices with clear error handling, informative logging, and proper file path resolution.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (10)
scripts/generate_openapi.py (10)
10-10: Remove unused imports.The
ListandOptionaltype hints are imported but never used in the code.-from typing import Dict, Any, List, Optional +from typing import Dict, Any
56-56: Rename unused loop variable.The loop control variable
iis not used within the loop body.-for i, match in enumerate(matches): +for _i, match in enumerate(matches):
37-44: Extract brace matching logic into a helper function.The brace matching logic is duplicated multiple times throughout the code. This makes maintenance difficult and increases the chance of inconsistent behavior.
269-270: Fix float detection logic.The current implementation incorrectly identifies strings like "1.2.3" as valid floats.
-elif value_str.replace(".", "").isdigit(): - return float(value_str) +else: + try: + return float(value_str) + except ValueError: + pass
275-275: Replace bare except clauses with specific exception handling.Bare
exceptstatements can mask unexpected errors and make debugging difficult.- except: + except (json.JSONDecodeError, ValueError): return []
280-280: Replace bare except clauses with specific exception handling.- except: + except (json.JSONDecodeError, ValueError): return []
286-286: Replace bare except clauses with specific exception handling.- except: + except (json.JSONDecodeError, ValueError): # Try to convert TypeScript object to JSON
291-291: Replace bare except clauses with specific exception handling.- except: + except (json.JSONDecodeError, ValueError): return {}
297-754: Break down the large function for better maintainability.The
generate_openapi_schemafunction is extremely large (over 450 lines) and handles multiple responsibilities including creating component schemas, generating evaluator-specific schemas, and creating path definitions.
607-684: Eliminate code duplication in path generation.The code for creating paths with settings and without settings is nearly identical, differing only in the request body schema. This creates maintenance overhead and potential for inconsistencies.
Also applies to: 687-752
🧹 Nitpick comments (1)
scripts/generate_openapi.py (1)
245-245: Rename unused loop variable.The loop control variable
iis not used within the loop body.-for i, char in enumerate(content[bracket_start:], bracket_start): +for _i, char in enumerate(content[bracket_start:], bracket_start):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/generate_openapi.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
scripts/generate_openapi.py
10-10: typing.List imported but unused
Remove unused import
(F401)
10-10: typing.Optional imported but unused
Remove unused import
(F401)
56-56: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
245-245: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
275-275: Do not use bare except
(E722)
280-280: Do not use bare except
(E722)
286-286: Do not use bare except
(E722)
291-291: Do not use bare except
(E722)
🔇 Additional comments (3)
scripts/generate_openapi.py (3)
1-1: LGTM: Proper shebang for Python script.The shebang line correctly specifies the Python interpreter for script execution.
757-789: LGTM: Well-structured main function with proper error handling.The main function properly handles file operations, error cases, and provides informative output messages.
763-763: Path resolution verifiedThe file
ts-integration/evaluators.generated.tsexists at the expected location. No further changes are needed.
scripts/generate_openapi.py
Outdated
| { | ||
| "lang": "python", | ||
| "label": "Python", | ||
| "source": f'import langwatch\n@langwatch.span()\ndef llm_step():\n ... # your existing code\n result = langwatch.get_current_span().evaluate(\n "{evaluator_id}",\n{chr(10).join(f" {field}=\"\", # required" for field in evaluator.get("requiredFields", []))}\n{chr(10).join(f" {field}=\"\", # optional" for field in evaluator.get("optionalFields", []))}\n settings={{}},\n )\n print(result)', |
There was a problem hiding this comment.
Fix inconsistent comment syntax in code samples.
The TypeScript code samples contain Python-style # comments instead of JavaScript-style // comments.
-{chr(10).join(f" {field}: \"\", # optional" for field in evaluator.get("optionalFields", []))}
+{chr(10).join(f" {field}: \"\", // optional" for field in evaluator.get("optionalFields", []))}Also applies to: 681-681, 743-743, 748-748
🤖 Prompt for AI Agents
In scripts/generate_openapi.py at lines 675, 681, 743, and 748, the code samples
use Python-style '#' comments which are inconsistent with the
TypeScript/JavaScript context. Replace all '#' comment markers in these code
samples with '//' to match JavaScript-style comments.
| # Quote property names (unquoted keys) | ||
| s = re.sub(r"([,{]\s*)([A-Za-z0-9_]+)\s*:", r'\1"\2":', s) | ||
| # Quote string values (only those not already quoted) | ||
| s = re.sub(r":\s*([A-Za-z0-9_\-/\.]+)", r': "\1"', s) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve TypeScript to JSON conversion robustness.
The regular expression patterns for converting TypeScript syntax to JSON may produce incorrect results in edge cases, particularly with nested objects or complex string values.
- # Quote property names (unquoted keys)
- s = re.sub(r"([,{]\s*)([A-Za-z0-9_]+)\s*:", r'\1"\2":', s)
- # Quote string values (only those not already quoted)
- s = re.sub(r":\s*([A-Za-z0-9_\-/\.]+)", r': "\1"', s)
+ # Quote property names (unquoted keys) - more precise pattern
+ s = re.sub(r"([,{]\s*)([A-Za-z_][A-Za-z0-9_]*)\s*:", r'\1"\2":', s)
+ # Quote string values (only those not already quoted) - avoid over-quoting
+ s = re.sub(r":\s*([A-Za-z_][A-Za-z0-9_\-/\.]*)\s*([,}\]])", r': "\1"\2', s)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Quote property names (unquoted keys) | |
| s = re.sub(r"([,{]\s*)([A-Za-z0-9_]+)\s*:", r'\1"\2":', s) | |
| # Quote string values (only those not already quoted) | |
| s = re.sub(r":\s*([A-Za-z0-9_\-/\.]+)", r': "\1"', s) | |
| # Quote property names (unquoted keys) - more precise pattern | |
| s = re.sub(r"([,{]\s*)([A-Za-z_][A-Za-z0-9_]*)\s*:", r'\1"\2":', s) | |
| # Quote string values (only those not already quoted) - avoid over-quoting | |
| s = re.sub(r":\s*([A-Za-z_][A-Za-z0-9_\-/\.]*)\s*([,}\]])", r': "\1"\2', s) |
🤖 Prompt for AI Agents
In scripts/generate_openapi.py around lines 207 to 210, the current regex
patterns for quoting property names and string values in TypeScript to JSON
conversion are too simplistic and may fail with nested objects or complex
strings. To fix this, replace the regex approach with a more robust parsing
method or improve the regex to correctly handle nested structures and escape
characters, ensuring all keys and string values are properly quoted without
affecting nested or complex values.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (9)
scripts/generate_openapi.py (9)
10-10: Remove unused importsListandOptional.These type hints are imported but never used in the code.
-from typing import Dict, Any, List, Optional +from typing import Dict, Any
56-56: Rename unused loop variableito_i.The loop control variable is not used within the loop body.
-for i, match in enumerate(matches): +for _i, match in enumerate(matches):
37-44: Extract repeated brace matching logic into a helper function.The brace matching logic is duplicated across multiple functions, making the code harder to maintain.
def find_matching_brace(content: str, start_pos: int, open_char: str = '{') -> int: """Find the position of the matching closing brace.""" close_char = '}' if open_char == '{' else ']' brace_count = 0 end_pos = start_pos for i, char in enumerate(content[start_pos:], start_pos): if char == open_char: brace_count += 1 elif char == close_char: brace_count -= 1 if brace_count == 0: end_pos = i + 1 break return end_posThen replace the duplicated logic with calls to this helper function.
Also applies to: 64-71
207-210: Improve TypeScript to JSON conversion robustness.The regular expression patterns for converting TypeScript syntax to JSON may produce incorrect results in edge cases, particularly with nested objects or complex string values.
- # Quote property names (unquoted keys) - s = re.sub(r"([,{]\s*)([A-Za-z0-9_]+)\s*:", r'\1"\2":', s) - # Quote string values (only those not already quoted) - s = re.sub(r":\s*([A-Za-z0-9_\-/\.]+)", r': "\1"', s) + # Quote property names (unquoted keys) - more precise pattern + s = re.sub(r"([,{]\s*)([A-Za-z_][A-Za-z0-9_]*)\s*:", r'\1"\2":', s) + # Quote string values (only those not already quoted) - avoid over-quoting + s = re.sub(r":\s*([A-Za-z_][A-Za-z0-9_\-/\.]*)\s*([,}\]])", r': "\1"\2', s)
269-270: Fix incorrect float detection logic.The current implementation would incorrectly identify strings like "1.2.3" as floats.
-elif value_str.replace(".", "").isdigit(): - return float(value_str) +else: + try: + return float(value_str) + except ValueError: + pass
275-275: Replace bare except clauses with specific exception handling.Bare
exceptstatements can mask unexpected errors and make debugging difficult.- except: + except (json.JSONDecodeError, ValueError): return []- except: + except (json.JSONDecodeError, ValueError): return {}Also applies to: 280-280, 286-286, 291-291
297-764: Break down this large function for better maintainability.This function spans over 400 lines and handles multiple responsibilities. Consider extracting helper functions for:
- Creating component schemas
- Generating evaluator-specific schemas
- Creating path definitions
463-689: Eliminate code duplication in path generation.The code for creating paths with settings (lines 463-689) and without settings (lines 691-762) is nearly identical, differing only in the request body schema.
Extract the common path generation logic into a helper function:
def create_evaluator_path(evaluator_id, evaluator, request_schema_name, has_settings=False, settings_schema_name=None): """Create path definition for an evaluator.""" path_def = { "post": { "summary": evaluator["name"], "description": evaluator["description"], "operationId": f"{evaluator_id.replace('.', '_').replace('/', '_')}_evaluate", "requestBody": { "content": { "application/json": { "schema": {"$ref": f"#/components/schemas/{request_schema_name}"} } }, "required": True, }, "responses": { "200": { "description": "Successful evaluation", "content": { "application/json": { "schema": { "type": "array", "items": {"$ref": "#/components/schemas/EvaluationResult"}, } } }, }, "400": { "description": "Bad request", "content": { "application/json": { "schema": { "type": "object", "properties": {"detail": {"type": "string"}}, } } }, }, "500": { "description": "Internal server error", "content": { "application/json": { "schema": { "type": "object", "properties": {"detail": {"type": "string"}}, } } }, }, }, } } # Modify schema for settings if needed if has_settings and settings_schema_name: path_def["post"]["requestBody"]["content"]["application/json"]["schema"] = { "allOf": [ {"$ref": f"#/components/schemas/{request_schema_name}"}, { "type": "object", "properties": { "settings": {"$ref": f"#/components/schemas/{settings_schema_name}"} }, }, ] } return path_defAlso applies to: 691-762
675-675: Fix inconsistent comment syntax in code samples.The TypeScript code samples contain Python-style
#comments instead of JavaScript-style//comments.-{chr(10).join(f" {field}: \"\", # optional" for field in evaluator.get("optionalFields", []))} +{chr(10).join(f" {field}: \"\", // optional" for field in evaluator.get("optionalFields", []))}Also applies to: 681-681, 743-743, 748-748
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/generate_openapi.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
scripts/generate_openapi.py
10-10: typing.List imported but unused
Remove unused import
(F401)
10-10: typing.Optional imported but unused
Remove unused import
(F401)
56-56: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
245-245: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
275-275: Do not use bare except
(E722)
280-280: Do not use bare except
(E722)
286-286: Do not use bare except
(E722)
291-291: Do not use bare except
(E722)
| open_char = content[bracket_start] | ||
| close_char = "]" if open_char == "[" else "}" | ||
|
|
||
| for i, char in enumerate(content[bracket_start:], bracket_start): |
There was a problem hiding this comment.
Rename unused loop variable i to _i.
The loop control variable is not used within the loop body.
-for i, char in enumerate(content[bracket_start:], bracket_start):
+for _i, char in enumerate(content[bracket_start:], bracket_start):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for i, char in enumerate(content[bracket_start:], bracket_start): | |
| for _i, char in enumerate(content[bracket_start:], bracket_start): |
🧰 Tools
🪛 Ruff (0.12.2)
245-245: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
🤖 Prompt for AI Agents
In scripts/generate_openapi.py at line 245, the loop variable `i` is declared
but not used inside the loop. Rename `i` to `_i` to indicate it is intentionally
unused and to follow Python conventions for unused variables.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (10)
scripts/generate_openapi.py (10)
10-10: Remove unused imports.The imports
ListandOptionalfrom typing are not used anywhere in the code.
56-56: Rename unused loop variableito_i.The loop control variable is not used within the loop body.
245-245: Rename unused loop variableito_i.The loop control variable is not used within the loop body.
269-270: Fix incorrect float detection logic.The current implementation would incorrectly identify strings like "1.2.3" as floats.
275-275: Replace bare except clauses with specific exception handling.Bare
exceptstatements can mask unexpected errors and make debugging difficult.Also applies to lines 280, 286, 291.
297-764: Break down this large function for better maintainability.This function spans over 400 lines and handles multiple responsibilities. Consider extracting helper functions for creating component schemas, generating evaluator-specific schemas, and creating path definitions.
607-689: Eliminate code duplication in path generation.The code for creating paths with settings (lines 607-689) and without settings (lines 692-762) is nearly identical, differing only in the request body schema.
685-685: Fix inconsistent comment syntax in code samples.The TypeScript code samples contain Python-style
#comments instead of JavaScript-style//comments.Also applies to lines 748, 758.
37-44: Extract repeated brace matching logic into a helper function.The brace matching logic is duplicated across multiple functions, making the code harder to maintain.
Also applies to lines 64-71, 143-150, 178-185, 245-252.
207-210: Improve TypeScript to JSON conversion robustness.The regular expression patterns for converting TypeScript syntax to JSON may produce incorrect results in edge cases, particularly with nested objects or complex string values.
🧹 Nitpick comments (5)
scripts/generate_openapi.py (1)
773-773: Use more robust path construction.The hardcoded path construction assumes a specific directory structure. Consider using a more flexible approach to locate the TypeScript file.
- evaluators_file = project_root / "ts-integration" / "evaluators.generated.ts" + # Try multiple possible locations + possible_paths = [ + project_root / "ts-integration" / "evaluators.generated.ts", + project_root / "src" / "evaluators.generated.ts", + project_root / "evaluators.generated.ts" + ] + evaluators_file = None + for path in possible_paths: + if path.exists(): + evaluators_file = path + break + + if not evaluators_file:scripts/generate_mdx_list.py (4)
9-9: Remove unused import.The import
Listfrom typing is not used anywhere in the code.-from typing import Dict, Any, List +from typing import Dict, Any
58-64: Move regex import to module level.Importing
reinside a function reduces readability and is inefficient since it's called for every evaluator.+import re import json import sys from pathlib import Path from typing import Dict, Any, List ... # Replace spaces and special characters with hyphens endpoint_id = endpoint_id.replace(" ", "-") endpoint_id = endpoint_id.replace("_", "-") endpoint_id = endpoint_id.replace("/", "-") # Remove any non-alphanumeric characters except hyphens - import re - endpoint_id = re.sub(r"[^a-z0-9\-]", "", endpoint_id)
74-125: Consider using a more maintainable categorization approach.The current keyword-based categorization logic uses hardcoded lists and multiple conditionals. Consider using a data-driven approach for better maintainability.
# Define categorization rules at module level CATEGORIZATION_RULES = { "Expected Answer Evaluation": [ "exact_match", "llm_answer_match", "factual", "sql_query", "rouge", "bleu" ], "LLM-as-Judge": [ "llm_boolean", "llm_score", "llm_category", "rubrics" ], "RAG Quality": [ "faithfulness", "context_precision", "context_recall", "context_f1", "response_relevancy", "response_context" ], "Quality Aspects Evaluation": [ "language_detection", "valid_format", "summarization" ], "Safety": [ "pii", "jailbreak", "prompt_injection", "content_safety", "moderation", "llama_guard" ] } def categorize_evaluator(evaluator_id: str) -> str: """Determine the category for a given evaluator ID.""" evaluator_lower = evaluator_id.lower() for category, keywords in CATEGORIZATION_RULES.items(): if any(keyword in evaluator_lower for keyword in keywords): return category return "Other"
160-198: Enhance error handling specificity.The generic exception handling could mask specific errors. Consider handling different error types more specifically.
try: # Find the openapi.json file script_dir = Path(__file__).parent openapi_file = script_dir / "openapi.json" if not openapi_file.exists(): print(f"Error: Could not find OpenAPI file at {openapi_file}") sys.exit(1) print(f"Reading OpenAPI from: {openapi_file}") with open(openapi_file, "r", encoding="utf-8") as f: openapi_data = json.load(f) + except FileNotFoundError: + print(f"Error: OpenAPI file not found at {openapi_file}") + sys.exit(1) + except json.JSONDecodeError as e: + print(f"Error: Invalid JSON in OpenAPI file: {e}") + sys.exit(1) + except PermissionError: + print(f"Error: Permission denied accessing {openapi_file}") + sys.exit(1) + try: paths = openapi_data.get("paths", {}) categories = categorize_evaluators(paths) mdx_content = generate_mdx(categories) # Write the MDX content to a file output_path = script_dir / "evaluators-list.mdx" with open(output_path, "w", encoding="utf-8") as f: f.write(mdx_content) print(f"MDX list generated successfully at: {output_path}") # Print summary total_evaluators = sum(len(cat["evaluators"]) for cat in categories.values()) active_categories = len( [cat for cat in categories.values() if cat["evaluators"]] ) print( f"Generated {total_evaluators} evaluators across {active_categories} categories" ) - except Exception as error: - print(f"Error generating MDX list: {error}") + except Exception as e: + print(f"Error generating MDX list: {e}") sys.exit(1)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/generate_mdx_list.py(1 hunks)scripts/generate_openapi.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
scripts/generate_openapi.py (1)
scripts/generate_mdx_list.py (1)
main(160-198)
🪛 Ruff (0.12.2)
scripts/generate_mdx_list.py
9-9: typing.List imported but unused
Remove unused import: typing.List
(F401)
scripts/generate_openapi.py
10-10: typing.List imported but unused
Remove unused import
(F401)
10-10: typing.Optional imported but unused
Remove unused import
(F401)
56-56: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
245-245: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
275-275: Do not use bare except
(E722)
280-280: Do not use bare except
(E722)
286-286: Do not use bare except
(E722)
291-291: Do not use bare except
(E722)
|
@richhuth consider implementing some of the code rabbit suggestions? |
Summary by CodeRabbit