Fix LLM filter cache key inputs#831
Conversation
Co-authored-by: Colin <Colin_XKL@outlook.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Code Review
This pull request refactors the LLM filter processor to generate cache keys based on the full prompt and the actual LLM payload rather than just the raw condition. It extracts the prompt construction logic into a helper function and adds a unit test to verify the new caching behavior. A review comment identifies a potential nil pointer dereference in the new cache key generator if the article is nil.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| return func(article *model.CraftArticle) (string, error) { | ||
| payload := "" | ||
| if payloadBuilder != nil { | ||
| payload = payloadBuilder(article) | ||
| } |
There was a problem hiding this comment.
The payloadBuilder function is called with article without verifying if article is nil. If article is nil, accessing article.Title inside the payload builder (as done in newLLMFilterProcessor) will cause a nil pointer dereference panic. Adding a defensive check for nil at the beginning of the generator function prevents this potential panic.
return func(article *model.CraftArticle) (string, error) {
if article == nil {
return "", fmt.Errorf("article is nil")
}
payload := ""
if payloadBuilder != nil {
payload = payloadBuilder(article)
}
Reviewer's GuideUpdates the LLM filter processor to generate cache keys from the fully rendered generic prompt plus the exact LLM article payload, and adds a regression test to ensure cached responses prevent redundant LLM calls. Flow diagram for updated LLM filter cache key and call pathflowchart TD
subgraph PromptPipeline
C[condition or userPrompt] --> G[buildGenericConditionPrompt]
G --> FP[full generic condition prompt]
FP --> PH[util.GetTextContentHash prompt]
end
subgraph PayloadPipeline
A[CraftArticle] --> P[BuildLLMArticlePayload]
P --> PL[LLM article payload]
PL --> LH[util.GetTextContentHash payload]
end
PH --> J[strings.Join promptHash or payloadHash]
LH --> J
J --> KH[util.GetTextContentHash cacheKey]
KH --> M[GetCommonCachedArticlePredicate]
M --> D{cache entry exists}
D -->|yes| R[cached predicate result]
D -->|no| L[CheckConditionWithLLM]
L --> R
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary
Testing
go test ./internal/craft -run TestLLMFilterProcessor_CacheKeyUsesFullPromptAndLLMPayload -count=1go test ./internal/craft -count=1task fixgo test ./...task backend-buildtask frontend-buildSummary by Sourcery
Improve LLM filter caching by keying on the full rendered prompt and article payload to prevent unnecessary LLM calls.
Bug Fixes:
Enhancements:
Tests: