fix: dedupe skill bodies & skip synthetic skill messages in prompt cache#231
Conversation
formatAgentMessages: add optional `skipSkillBodyNames` so historical `skill` tool_calls are not reconstructed into a HumanMessage when the same skill is already primed fresh this turn — preventing the SKILL.md body from being injected twice. Names absent from the set still reconstruct from history. cache: add `isSyntheticMetaMessage` and skip those messages when anchoring fresh prompt-cache markers in addCacheControl (Anthropic/OpenRouter) and addBedrockCacheControl. Stale markers are still stripped; the marker moves to the next real user message instead of pinning to a volatile skill prefix. Adds tests for both behaviors.
|
Companion (consumer) PR: danny-avila/LibreChat#13610 |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5954edbdd6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const needsCacheAdd = | ||
| userMessagesModified < 2 && | ||
| isUserMessage && | ||
| !isSyntheticMetaMessage(originalMessage) && |
There was a problem hiding this comment.
This guard only covers the direct addCacheControl path; the stable-prefix path used by AgentContext.buildBodyWithPromptCacheDynamicTail still calls addCacheControlToStablePrefixMessages, whose fallback delegates to addCacheControlToRecentMessages without checking isSyntheticMetaMessage. When prompt caching has a dynamic tail (dynamic instructions/summary) and the stable prefix contains a reconstructed skill HumanMessage after a skill-only assistant tool call, the assistant-only pass can add no marker and the fallback will still cache-anchor the synthetic SKILL.md body, so the duplicated/volatile prefix this change is trying to avoid remains possible.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in ddd1ddf. The guard now lives in addCacheControlToRecentMessages (the shared backend for addCacheControlToStablePrefixMessages), so both the assistant pass and the cacheable fallback skip synthetic skill/meta messages. Added a regression test that reproduces the dynamic-tail scenario: a skill-only (text-less) assistant tool call followed by a reconstructed skill HumanMessage in the stable prefix — the fallback now anchors the real user message instead of the SKILL.md body.
Addresses Codex P2: addCacheControlToStablePrefixMessages (the dynamic-tail path used by AgentContext) delegates to addCacheControlToRecentMessages, whose cacheable fallback did not check isSyntheticMetaMessage. When the stable prefix held a reconstructed skill HumanMessage after a skill-only (text-less) assistant tool call, the fallback could cache-anchor the synthetic SKILL.md body. Guard canAddCache with !isSyntheticMetaMessage so both the assistant pass and the cacheable fallback skip synthetic skill/meta messages. Adds a regression test for the stable-prefix fallback.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Problem
When a skill is primed fresh on the current turn (LibreChat splices the SKILL.md body in just before the latest user message) and the same skill also appears earlier in history as a
skilltool_call,formatAgentMessagesreconstructs the historical body into a syntheticHumanMessage— so the body appears twice. Prompt-cache markers could then land on those synthetic skill-prime messages, pinning the cache to a duplicated/volatile prefix.Changes
formatAgentMessages—skipSkillBodyNamesskipSkillBodyNames?: Set<string>onFormatAgentMessagesOptions.skilltool_calls whose name is in the set are not reconstructed into aHumanMessage. Names absent from the set still reconstruct, preserving sticky re-priming across turns.prompt cache — skip synthetic skill/meta messages
isSyntheticMetaMessage(message)helper:additional_kwargs.isMeta === true || additional_kwargs.source === 'skill'.addCacheControl(Anthropic / OpenRouter) andaddBedrockCacheControlno longer add fresh cache markers to synthetic skill/meta messages; the marker moves to the next real user message.Tests
formatAgentMessages.skills.test.ts: reconstruct skip when name is inskipSkillBodyNames, selective skip, empty-set no-op, non-tools-filtering path.cache.test.ts: Anthropic + Bedrock skip synthetic skillHumanMessage, marker moves to the next real message, stale marker stripped without re-add,source-only detection.78 tests pass across both files; lint + typecheck clean (only pre-existing unrelated warnings).
Companion
Paired with the LibreChat change that passes
skipSkillBodyNames(manual + always-apply prime names) intoformatAgentMessages.