-
Notifications
You must be signed in to change notification settings - Fork 24
Description
Context
Several CodeRabbit findings from PR #531 relate to generated code (builtin_evaluators.go) or the generator scripts / SDK that produce it. These require changes to the Python SDK evaluators + regeneration, or behavioral changes that warrant their own review cycle.
Issues to address
Generated code (builtin_evaluators.go — generated by scripts/generate-builtin-evaluators.sh)
-
Guard zero/negative task constraints before division (Major)
- Lines 34, 131: Division by
max_iterations/max_tokenscan raiseZeroDivisionErrorif a task overrides with 0 or negative. - Fix: Add guard
if max_iterations is None or max_iterations <= 0in the Python SDK evaluator source.
- Lines 34, 131: Division by
-
instruction_followingprompt drops success criteria (Minor)- Line 338: Prints
Success criteria:but doesn't interpolate the value.
- Line 338: Prints
-
Literal
\ninstead of real newlines in prompt text (Minor)- Lines 357, 376:
\\nTask:renders as literal backslash-n, not a line break.
- Lines 357, 376:
-
contextconfig defined but not interpolated in Safety/Tone prompts (Major)- Lines 414, 434:
contextconfig param exists but prompt renders emptyContext:.
- Lines 414, 434:
Generator script (generate-builtin-evaluators.sh)
-
Prompt template extraction silently drops valid
build_promptimplementations (Major)- Line 181: Only extracts f-string return values;
return promptpatterns produce emptySource.
- Line 181: Only extracts f-string return values;
-
Output directory not created before heredoc redirection (Major)
- Line 457:
LLM_JUDGE_BASE_FILEpath may not exist if--outputpoints to custom location.
- Line 457:
SDK (amp_evaluation)
-
Enforce required function-level
Params during initialization (Major,base.py:740)_init_function_params()doesn't raise when a requiredParam(no default) isn't provided, delaying errors to runtime.
-
f-string docs clarification (Minor,
evaluator_schema.py:1356)- AI copilot guide says "loops" are supported inside
{}— only comprehension expressions work, not loop statements.
- AI copilot guide says "loops" are supported inside