Lorenze/imp/prompt layering#5774
Merged
Merged
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b2488ec. Configure here.
📝 WalkthroughWalkthroughAdds provider-agnostic prompt cache-breakpoint markers, integrates them into executors and base LLM message prep (markers stripped before provider submission), implements Anthropic ephemeral stamping, replaces Markdown skill headers with XML ChangesPrompt Caching and Skill Context Refactor
Sequence DiagramsequenceDiagram
participant AgentExec as Agent Executor
participant Prompts as Prompts Builder
participant ExecutorState as Message State
participant LLMBase as BaseLLM
participant Anthropic as Anthropic Provider
AgentExec->>Prompts: task_execution() -> render prompt + _build_skill_block()
Prompts-->>AgentExec: formatted system/user prompt (+ <skills> block)
AgentExec->>ExecutorState: append mark_cache_breakpoint(formatted_message)
ExecutorState->>LLMBase: prepare_messages(marked_messages)
LLMBase->>LLMBase: create cleaned copies (strip CACHE_BREAKPOINT_KEY)
LLMBase->>Anthropic: _format_messages_for_anthropic(cleaned_messages)
Anthropic->>Anthropic: detect original markers and stamp cache_control ephemeral
Anthropic-->>AgentExec: formatted payload (with ephemeral stamps where applicable)
🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels: Suggested reviewers:
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/crewai/src/crewai/skills/loader.py (1)
175-188:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEscape XML-sensitive content before building
<skill>blocksLine 175 and Line 188 inject
skill.namedirectly into an XML attribute, and Lines 176/178 inject raw text into the wrapped body. If any field contains",<,&, or</skill>, it can break block boundaries and destabilize the prompt/cache anchor format.Suggested fix
+from html import escape + def format_skill(skill: Skill) -> str: + safe_name = escape(skill.name or "", quote=True) + safe_description = escape(skill.description or "") + safe_instructions = escape(skill.instructions or "") + if skill.disclosure_level >= INSTRUCTIONS and skill.instructions: parts = [ - f'<skill name="{skill.name}">', - skill.description, + f'<skill name="{safe_name}">', + safe_description, "", - skill.instructions, + safe_instructions, ] ... parts.append("</skill>") return "\n".join(parts) - return f'<skill name="{skill.name}">\n{skill.description}\n</skill>' + return f'<skill name="{safe_name}">\n{safe_description}\n</skill>'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/skills/loader.py` around lines 175 - 188, Escape XML-sensitive characters before injecting skill fields into the generated <skill> blocks: locate the code in lib/crewai/src/crewai/skills/loader.py that constructs the skill XML (the function that concatenates or formats skill.name and the skill body/text into the "<skill ...>...</skill>" block) and replace raw inserts with an XML-escaping step (e.g., escape &, <, >, " and apostrophe) for attribute values and either escape or wrap body text in a safe container (CDATA) for the element content; ensure the code that references skill.name and the wrapped body text uses the escaped values so that quotes, angle brackets, ampersands, or literal "</skill>" cannot break the block boundaries.
🧹 Nitpick comments (2)
lib/crewai/src/crewai/utilities/prompts.py (1)
89-89: ⚡ Quick winStabilize and reuse the skill block once per prompt build.
_build_skill_block()is called repeatedly, and the block order currently depends on incomingagent.skillsorder. Sorting by skill name and computing once intask_execution()makes cache-prefix text deterministic and avoids duplicate formatting work.♻️ Proposed refactor
@@ - system: str = self._build_prompt(slices) + self._build_skill_block() + skill_block = self._build_skill_block() + system: str = self._build_prompt(slices) + skill_block @@ system=system, user=self._build_prompt([task_slice]), - prompt=self._build_prompt(slices) + self._build_skill_block(), + prompt=self._build_prompt(slices) + skill_block, ) return StandardPromptResult( prompt=self._build_prompt( @@ self.prompt_template, self.response_template, ) - + self._build_skill_block() + + skill_block ) @@ - sections = [format_skill_context(s) for s in skills if isinstance(s, Skill)] + stable_skills = sorted( + (s for s in skills if isinstance(s, Skill)), + key=lambda s: s.name, + ) + sections = [format_skill_context(s) for s in stable_skills] if not sections: return ""Also applies to: 109-109, 118-118, 121-137
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/src/crewai/utilities/prompts.py` at line 89, The code repeatedly calls _build_skill_block() and relies on agent.skills' incoming order, causing nondeterministic cache prefixes and wasted formatting; compute the skill block once (e.g., in task_execution()) by sorting agent.skills by skill.name to produce a deterministic ordered list, build the skill_block string there, store it in a local variable (e.g., skill_block) and replace all direct calls to _build_skill_block() (including where system is set and other uses of _build_skill_block) to reuse that stored skill_block so formatting is done only once per prompt build.lib/crewai/tests/llms/test_prompt_cache.py (1)
67-69: ⚡ Quick winAvoid position-dependent block selection in Anthropic assertion.
formatted[0]["content"][-1]can become brittle if content/message ordering shifts. Prefer locating the specifictextblock by role/type/content, like you already do in the next test.Proposed test hardening
- # First user block carries cache_control too - last_block = formatted[0]["content"][-1] - assert last_block["cache_control"] == {"type": "ephemeral"} + # Stable user text block carries cache_control too + user_msg = next(fm for fm in formatted if fm["role"] == "user") + text_block = next( + b + for b in user_msg["content"] + if isinstance(b, dict) + and b.get("type") == "text" + and b.get("text") == "ping" + ) + assert text_block.get("cache_control") == {"type": "ephemeral"}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/tests/llms/test_prompt_cache.py` around lines 67 - 69, The assertion is brittle because it picks the last content entry via formatted[0]["content"][-1]; instead, locate the specific user/text block deterministically (e.g., iterate formatted[0]["content"] and find the dict whose "type" or "text"/"role" matches the expected user message) and then assert that its "cache_control" equals {"type": "ephemeral"}—update the test in test_prompt_cache.py to search for the block by its identifying fields rather than by position.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/crewai/src/crewai/llms/base_llm.py`:
- Around line 722-729: The code that converts incoming dicts into LLMMessage
instances (in BaseLLM, the method that currently casts messages to LLMMessage)
only checks for key presence and should also validate value types: ensure "role"
is a str and "content" is a str (or the expected type), otherwise raise a
ValueError immediately instead of casting; update the message-casting routine
(the BaseLLM message conversion helper that produces LLMMessage) to perform
these type checks before constructing LLMMessage and include a clear ValueError
when types are invalid.
In `@lib/crewai/src/crewai/llms/providers/anthropic/completion.py`:
- Around line 704-707: The code that collects message content in the Anthropic
completion path is currently only grabbing string content from "assistant"
messages and the later stamping loop (in the same module) only applies
cache_breakpoint handling to "user" messages, so assistant cache_breakpoint
flags and structured list-form content are ignored; update the collection step
(where message content is aggregated) to (1) stop collecting/depending on
"assistant" roles if they are not stamped, or better remove the unused assistant
collection, and (2) when extracting content from a message (look for the place
that reads message["content"] and sets collected content), handle both string
and list-form content by detecting if content is a list and
concatenating/extracting text fields (preserving original ordering) so messages
with {"content": [...], "cache_breakpoint": True} are honored; ensure the
stamping loop that sets cache_breakpoint (the loop that checks message["role"]
== "user") either also checks "assistant" roles or you remove assistant
collection so only user roles are processed, and keep the cache_breakpoint flag
propagation logic consistent with these changes.
---
Outside diff comments:
In `@lib/crewai/src/crewai/skills/loader.py`:
- Around line 175-188: Escape XML-sensitive characters before injecting skill
fields into the generated <skill> blocks: locate the code in
lib/crewai/src/crewai/skills/loader.py that constructs the skill XML (the
function that concatenates or formats skill.name and the skill body/text into
the "<skill ...>...</skill>" block) and replace raw inserts with an XML-escaping
step (e.g., escape &, <, >, " and apostrophe) for attribute values and either
escape or wrap body text in a safe container (CDATA) for the element content;
ensure the code that references skill.name and the wrapped body text uses the
escaped values so that quotes, angle brackets, ampersands, or literal "</skill>"
cannot break the block boundaries.
---
Nitpick comments:
In `@lib/crewai/src/crewai/utilities/prompts.py`:
- Line 89: The code repeatedly calls _build_skill_block() and relies on
agent.skills' incoming order, causing nondeterministic cache prefixes and wasted
formatting; compute the skill block once (e.g., in task_execution()) by sorting
agent.skills by skill.name to produce a deterministic ordered list, build the
skill_block string there, store it in a local variable (e.g., skill_block) and
replace all direct calls to _build_skill_block() (including where system is set
and other uses of _build_skill_block) to reuse that stored skill_block so
formatting is done only once per prompt build.
In `@lib/crewai/tests/llms/test_prompt_cache.py`:
- Around line 67-69: The assertion is brittle because it picks the last content
entry via formatted[0]["content"][-1]; instead, locate the specific user/text
block deterministically (e.g., iterate formatted[0]["content"] and find the dict
whose "type" or "text"/"role" matches the expected user message) and then assert
that its "cache_control" equals {"type": "ephemeral"}—update the test in
test_prompt_cache.py to search for the block by its identifying fields rather
than by position.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dd35940f-39d2-441b-b2b2-d6780d4e8c39
📒 Files selected for processing (12)
lib/crewai/src/crewai/agent/core.pylib/crewai/src/crewai/agent/utils.pylib/crewai/src/crewai/agents/crew_agent_executor.pylib/crewai/src/crewai/experimental/agent_executor.pylib/crewai/src/crewai/llms/base_llm.pylib/crewai/src/crewai/llms/cache.pylib/crewai/src/crewai/llms/providers/anthropic/completion.pylib/crewai/src/crewai/skills/loader.pylib/crewai/src/crewai/utilities/prompts.pylib/crewai/tests/llms/test_prompt_cache.pylib/crewai/tests/skills/test_integration.pylib/crewai/tests/skills/test_loader.py
💤 Files with no reviewable changes (2)
- lib/crewai/src/crewai/agent/core.py
- lib/crewai/src/crewai/agent/utils.py
…ewAI into lorenze/imp/prompt-layering
This was referenced May 17, 2026
This was referenced May 29, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Note
Medium Risk
Touches core prompt/message formatting and Anthropic provider payload shaping; mistakes could change prompts sent to LLMs or degrade caching behavior across tool-use loops.
Overview
Introduces a provider-agnostic
cache_breakpointmarker (crewai.llms.cache) and updates bothCrewAgentExecutorandexperimental.AgentExecutorto mark the stable system/user prompt prefix for reuse across ReAct/tool iterations.Updates
BaseLLM._format_messagesto strip breakpoint flags without mutating the caller’s message buffer, and extends the Anthropic provider to translate marked breakpoints intocache_control(including converting cached system prompts to content blocks and stamping only the intended stable user block).Moves skills prompt injection out of
Agentruntime prompt concatenation by removingappend_skill_context, emitting skill context as stable XML blocks (<skills>,<skill ...>) during prompt construction, and updates/adding tests to cover caching semantics and the new skill formatting.Reviewed by Cursor Bugbot for commit 6f94c9b. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Tests