Normalize cache keys on getWorkflow to workflow id#3604
Open
vegaro wants to merge 2 commits into
Open
Conversation
…ache misses When getWorkflow is called with an offering ID (no map entry yet), the backend lazily converts it and returns a workflow with a different real workflow ID. The result was only cached under the offering ID, so after the list was fetched and the offeringId→workflowId map populated, lookups by the real ID missed. Now the resolved result is cached once, under result.workflow.id, and the discovered offeringId→workflowId mapping is recorded via a new WorkflowsCache.recordWorkflowIdForOfferingId so the next workflowIdForOfferingId lookup resolves to that same single cache entry — instead of keying one workflow under two keys that then drift on invalidation/SWR refresh. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4f6ccbf to
7d15161
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3604 +/- ##
==========================================
+ Coverage 80.26% 80.27% +0.01%
==========================================
Files 378 378
Lines 15448 15456 +8
Branches 2143 2146 +3
==========================================
+ Hits 12400 12408 +8
Misses 2189 2189
Partials 859 859 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
3ecf84a to
297b27b
Compare
getWorkflow to workflow id
…ache misses The render path asks for a workflow by offering id (the backend lazily converts it), while prefetch asks by workflow id. getWorkflow's `workflowId` parameter was therefore overloaded, and the cache was keyed inconsistently: a workflow prefetched under its workflow id missed when later requested by offering id, and vice versa. WorkflowManager.getWorkflow now normalizes the incoming identifier through the offeringId→workflowId map to the canonical workflow id before the cache read, so an ask by either id hits the same entry (parameter renamed to `workflowOrOfferingId` to make the contract honest). On a successful resolve the result is cached once, under the resolved workflow's own id (via a shared cacheResolvedWorkflow helper), and any newly discovered offeringId→workflowId mapping is recorded (new WorkflowsCache.recordWorkflowIdForOfferingId) so subsequent asks by offering id resolve to it. A response that resolves with an empty id can't be keyed, so it is delivered for the current render but not cached under a wrong key. The disk-fallback path (resolveDiskFallback) caches the recovered envelope through the same helper, keying it off the resolved id too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
297b27b to
5d985a7
Compare
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
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.
Checklist
purchases-iosand hybridsMotivation
The render path asks for a workflow by offering id (the backend lazily converts it into a workflow), while the prefetch path asks by workflow id.
WorkflowManager.getWorkflow'sworkflowIdparameter was therefore overloaded — sometimes a workflow id, sometimes an offering id — and the cache was keyed inconsistently. A workflow prefetched under its workflow id missed when the render later asked by offering id, and a lazily-converted workflow cached under the offering id missed once the workflows list populated theofferingId → workflowIdmap. Either way: a redundant fetch on every render.Description
getWorkflownow normalizes the incoming identifier to the canonical workflow id (via theofferingId → workflowIdmap) before the cache read, so an ask by either id resolves to the same entry. The parameter is renamed toworkflowOrOfferingIdto make that contract honest. An as-yet-unconverted offering id has no mapping and passes through; its real id is learned from the response.On a successful resolve the result is cached once, under the resolved workflow's own id (via a shared
cacheResolvedWorkflowhelper), and any newly discoveredofferingId → workflowIdmapping is recorded (newWorkflowsCache.recordWorkflowIdForOfferingId, copy-on-write under the existing lock) so later asks by offering id resolve to it. A response that resolves with an empty id can't be keyed in the cache, so it's delivered for the current render but not cached under a wrong key — the next render refetches. The disk-fallback path (resolveDiskFallback) caches the recovered envelope through the same helper, so it keys off the resolved id too.No public API change. Unit tests cover: ask-by-offering-id hitting the entry prefetched under the workflow id; the lazy-conversion single-entry caching + mapping record + cache-hit on the next offering-id ask; the empty-id response being delivered but not cached; the disk-fallback re-pin keying off the resolved id; and the
WorkflowsCachemap record/replace behavior.Follow-ups (not in this PR)
purchases-iosand hybrids.Note
Medium Risk
Touches paywall workflow fetch/cache behavior (stale-while-revalidate and disk fallback), but changes are localized, backward-compatible at the public API, and covered by new unit tests.
Overview
Fixes redundant workflow fetches when the render path uses an offering id but prefetch/cache used a workflow id (or the opposite after lazy conversion).
WorkflowManager.getWorkflownow takesworkflowOrOfferingId, resolves it through the existing offering→workflow map before any cache read, and centralizes post-resolve storage incacheResolvedWorkflow: one in-memory entry under the resolved workflow id, optionalrecordWorkflowIdForOfferingIdwhen the request id differed, and no cache write when the resolved id is empty. Disk fallback re-pins through the same helper so keys stay consistent.No public SDK API change; orchestrator still exposes
workflowIdbut forwards the value asworkflowOrOfferingId. Tests cover prefetch-by-workflow-id vs render-by-offering-id, lazy conversion mapping, empty-id responses, and disk fallback keying.Reviewed by Cursor Bugbot for commit 5d985a7. Bugbot is set up for automated code reviews on this repo. Configure here.