fix(cache): invalidate cached responses when .intentrc changes#27
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e059c2a4a
ℹ️ 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".
| in.BinariesFingerprint, | ||
| in.BackendIdentity, | ||
| in.PromptTemplateVersion, | ||
| in.ProjectRCFingerprint, |
There was a problem hiding this comment.
Preserve baseline key when project directives are absent
Adding in.ProjectRCFingerprint to parts unconditionally changes the serialized key payload for every request (an extra field separator is always inserted), so repositories without .intentrc no longer reuse pre-existing cache entries. That defeats the stated goal of keeping baseline behavior unchanged and causes a broad cache miss wave unrelated to directive changes.
Useful? React with 👍 / 👎.
| trimmed := strings.TrimSpace(contents) | ||
| if trimmed == "" { | ||
| return "" | ||
| } | ||
| h := sha256.Sum256([]byte(trimmed)) |
There was a problem hiding this comment.
Hash non-blank .intentrc content without trimming semantics
ProjectRCFingerprint hashes strings.TrimSpace(contents), but the engine injects the original opts.ProjectRC into the system prompt. If .intentrc changes only in leading/trailing whitespace, the prompt text changes while the cache key does not, which can return a cached response generated under different prompt bytes. Only whitespace-only files should collapse to empty; non-blank content should be fingerprinted from the raw bytes.
Useful? React with 👍 / 👎.
Non-technical summary
This fixes a correctness bug where
intentcould reuse a cached response even after a repo's.intentrcdirectives changed. That mattered because project directives are part of how the model interprets a request, so stale cache reuse could makeintentbehave as if it were still following old repo-specific rules.This matters now because the cache is meant to make repeated requests faster without changing what the user asked the tool to do. After this change, editing
.intentrcreliably causesintentto compute a fresh response instead of replaying one generated under different project guidance.Technical summary
ProjectRCFingerprinttocache.KeyInputscache.ProjectRCFingerprintso non-empty.intentrccontent participates in cache identity.intentrccontent to an empty fingerprint so repos without real directives keep the baseline key behaviorengine.Runto include the.intentrcfingerprint when computing cache keysinternal/engine/engine_test.gofor:.intentrcdirectives change.intentrccontents.intentrccontentRelevant intent being strengthened: deterministic, honest behavior when repo-level directives change, consistent with the spec and with issue #19's acceptance criteria.
Tests:
go test ./internal/cache ./internal/enginego test ./...go vet ./...PATH="/opt/homebrew/bin:$PATH" make buildBreaking changes: none intended. Cached entries generated under old
.intentrccontents will naturally stop matching once project directives change, which is the desired bug fix.Additional notes
Trade-off: this widens cache identity slightly, but only for a prompt input that already materially affects model output.
Intentionally deferred:
.intentrcparticipationRemaining gap to the fuller vision: cache correctness still depends on every prompt-shaping input being accounted for, so other future prompt inputs should be reviewed with the same standard.
Closes #19.