feat: improve tip generation prompt with richer guidance#124
feat: improve tip generation prompt with richer guidance#124jayaramkr wants to merge 1 commit intoAgentToolkit:mainfrom
Conversation
Restructures the prompt to produce more specific, actionable tips by adding explicit categories (strategy/recovery/optimization), step instructions, trigger conditions, and domain-specific pattern hints (API discovery, pagination, auth, error handling).
📝 WalkthroughWalkthroughUpdated the Jinja2 prompt template to reframe tip generation from generic extraction to actionable, task-specific guidance. Added structured tip requirements including content, rationale, category classification, implementation steps, and trigger conditions. Incorporated branching logic for success-focused versus failure-recovery strategies and expanded the checklist of trajectory aspects to analyze. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
evolve/llm/tips/prompts/generate_tips.jinja2 (2)
48-51: Extraneous trailing newlines.The template ends with 3 blank lines after
{% endif %}. This adds unnecessary whitespace to the rendered prompt.Remove extra newlines
Generate tips now. Return ONLY the JSON, no other text. {% endif %} - - -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evolve/llm/tips/prompts/generate_tips.jinja2` around lines 48 - 51, The Jinja2 template ends with three extraneous blank lines after the `{% endif %}` which add unwanted whitespace to rendered prompts; remove the extra newline characters so the file ends immediately after the `{% endif %}` (ensure the trailing newline is exactly one or none per project style) in the generate_tips.jinja2 template to prevent extra whitespace in outputs.
20-20: Minor terminology inconsistency: "trigger condition" vs "trigger".Line 20 says "A trigger condition" but the JSON example on line 41 uses
"trigger"as the field name. While likely not a blocking issue, aligning the wording prevents ambiguity.Suggested wording alignment
- - A trigger condition (when to apply this tip) + - A trigger (when to apply this tip)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evolve/llm/tips/prompts/generate_tips.jinja2` at line 20, Update the wording to match the JSON field name: replace the phrase "A trigger condition (when to apply this tip)" with "A trigger (when to apply this tip)" in the generate_tips.jinja2 template so it aligns with the "trigger" field used in the JSON example; ensure any other occurrences in the same template use "trigger" for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@evolve/llm/tips/prompts/generate_tips.jinja2`:
- Around line 22-23: The template guidance refers to task success/failure but
the template is always rendered with "Status: UNKNOWN" because
evolve/llm/tips/tips.py (around the render function at lines 106-125) does not
pass the task outcome; fix by adding a boolean (e.g., task_succeeded) or outcome
string to the context when calling the Jinja2 render in the function that
renders generate_tips.jinja2 and update line 5 in generate_tips.jinja2 to use
that variable (e.g., "**Status:** {{ 'SUCCEEDED' if task_succeeded else 'FAILED'
}}") so the conditional guidance is meaningful, or if you prefer not to change
call sites, remove/rephrase the conditional lines 5-6 in generate_tips.jinja2 to
avoid referencing task success/failure.
- Around line 15-20: The template asks for "implementation_steps" but the Tip
Pydantic model lacks that field, causing validation failures in tips.py; fix by
either removing the "Specific steps to implement the tip" line from
generate_tips.jinja2 or by adding implementation_steps: list[str] to the Tip
model (class Tip) and then update any consumers (the JSON example in the
template and the clustering logic in clustering.py) and ensure tips.py
validation accepts the new field; reference symbols: Tip, implementation_steps,
generate_tips.jinja2, tips.py, clustering.py.
---
Nitpick comments:
In `@evolve/llm/tips/prompts/generate_tips.jinja2`:
- Around line 48-51: The Jinja2 template ends with three extraneous blank lines
after the `{% endif %}` which add unwanted whitespace to rendered prompts;
remove the extra newline characters so the file ends immediately after the `{%
endif %}` (ensure the trailing newline is exactly one or none per project style)
in the generate_tips.jinja2 template to prevent extra whitespace in outputs.
- Line 20: Update the wording to match the JSON field name: replace the phrase
"A trigger condition (when to apply this tip)" with "A trigger (when to apply
this tip)" in the generate_tips.jinja2 template so it aligns with the "trigger"
field used in the JSON example; ensure any other occurrences in the same
template use "trigger" for consistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 419fa8fe-8b10-488f-9af0-6c9679f136f5
📒 Files selected for processing (1)
evolve/llm/tips/prompts/generate_tips.jinja2
| 4. Each tip should have: | ||
| - A clear, concise description (content) | ||
| - The purpose/benefit of following it | ||
| - The category: "strategy", "recovery", or "optimization" | ||
| - Specific steps to implement the tip | ||
| - A trigger condition (when to apply this tip) |
There was a problem hiding this comment.
Schema mismatch: implementation_steps field does not exist in the Tip model.
The instructions ask the LLM to include "Specific steps to implement the tip" (line 19), but the Tip Pydantic model in evolve/schema/tips.py only defines 4 fields: content, rationale, category, and trigger. If the LLM emits an implementation_steps field, it will either cause a ValidationError (returning an empty tips list per evolve/llm/tips/tips.py:154-162) or be silently discarded.
Either remove the instruction for implementation steps, or update the Tip schema to include the new field.
Option A: Remove the mismatched instruction
4. Each tip should have:
- A clear, concise description (content)
- The purpose/benefit of following it
- The category: "strategy", "recovery", or "optimization"
- - Specific steps to implement the tip
- A trigger condition (when to apply this tip)Option B: Update the Tip schema (in evolve/schema/tips.py)
class Tip(BaseModel):
content: str = Field(description="Clear, actionable tip")
rationale: str = Field(description="Why this tip helps")
category: Literal["strategy", "recovery", "optimization"]
implementation_steps: list[str] = Field(description="Specific steps to implement this tip")
trigger: str = Field(description="When to apply this tip")If you choose Option B, also update the JSON example in this template (lines 35-44) and the clustering code in evolve/llm/tips/clustering.py.
📝 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.
| 4. Each tip should have: | |
| - A clear, concise description (content) | |
| - The purpose/benefit of following it | |
| - The category: "strategy", "recovery", or "optimization" | |
| - Specific steps to implement the tip | |
| - A trigger condition (when to apply this tip) | |
| 4. Each tip should have: | |
| - A clear, concise description (content) | |
| - The purpose/benefit of following it | |
| - The category: "strategy", "recovery", or "optimization" | |
| - A trigger condition (when to apply this tip) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@evolve/llm/tips/prompts/generate_tips.jinja2` around lines 15 - 20, The
template asks for "implementation_steps" but the Tip Pydantic model lacks that
field, causing validation failures in tips.py; fix by either removing the
"Specific steps to implement the tip" line from generate_tips.jinja2 or by
adding implementation_steps: list[str] to the Tip model (class Tip) and then
update any consumers (the JSON example in the template and the clustering logic
in clustering.py) and ensure tips.py validation accepts the new field; reference
symbols: Tip, implementation_steps, generate_tips.jinja2, tips.py,
clustering.py.
| 5. If the task succeeded, focus on the successful strategies used | ||
| 6. If the task failed, focus on what went wrong and how to prevent/recover from it |
There was a problem hiding this comment.
Guidance references task success/failure, but status is always "UNKNOWN".
Lines 22-23 instruct the LLM to focus on successful strategies if the task succeeded, or on failure modes if it failed. However, line 5 shows **Status:** UNKNOWN, and the rendering code in evolve/llm/tips/tips.py:106-125 does not pass a task_succeeded or similar variable to the template.
The LLM cannot follow this conditional guidance without knowing the actual outcome. Either:
- Pass the task outcome to the template and update line 5, or
- Remove/rephrase this guidance to avoid confusing the LLM.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@evolve/llm/tips/prompts/generate_tips.jinja2` around lines 22 - 23, The
template guidance refers to task success/failure but the template is always
rendered with "Status: UNKNOWN" because evolve/llm/tips/tips.py (around the
render function at lines 106-125) does not pass the task outcome; fix by adding
a boolean (e.g., task_succeeded) or outcome string to the context when calling
the Jinja2 render in the function that renders generate_tips.jinja2 and update
line 5 in generate_tips.jinja2 to use that variable (e.g., "**Status:** {{
'SUCCEEDED' if task_succeeded else 'FAILED' }}") so the conditional guidance is
meaningful, or if you prefer not to change call sites, remove/rephrase the
conditional lines 5-6 in generate_tips.jinja2 to avoid referencing task
success/failure.
|
@jayaramkr Can you address the code rabbit's comments, or mark them as irrelevant? |
Restructures the prompt to produce more specific, actionable tips by adding explicit categories (strategy/recovery/optimization), step instructions, trigger conditions, and domain-specific pattern hints (API discovery, pagination, auth, error handling).
Summary by CodeRabbit