fix: cache compiled programs by predeclared name set (SLT-17)#73
Conversation
Cache.Run keyed c.scripts by filename only, but SourceProgramOptions resolves each identifier as predeclared vs global against the current global-name set (dict.Has). Running the same file under a different set of globals reused a stale program: it failed at run time with "internal error: predeclared variable X is uninitialized", or silently reused a program that was resolved under the wrong name set. Fold the sorted predeclared name set into the cache key (the dialect is fixed, so filename + names suffice) and update Forget to drop every key belonging to a filename. Mirrors starlet's exec.go getCacheKey. Test-first: TestCachePredeclaredNameSet reproduces the stale-program internal error before the fix. Backlog: SLT-17. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 14 |
| Duplication | 0 |
🟢 Coverage 100.00% diff coverage · +0.16% coverage variation
Metric Results Coverage variation ✅ +0.16% coverage variation (-1.00%) Diff coverage ✅ 100.00% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (300e254) 2124 1915 90.16% Head commit (3da891e) 2139 (+15) 1932 (+17) 90.32% (+0.16%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#73) 19 19 100.00% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
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.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #73 +/- ##
==========================================
+ Coverage 87.12% 87.32% +0.19%
==========================================
Files 8 8
Lines 1662 1672 +10
==========================================
+ Hits 1448 1460 +12
+ Misses 131 130 -1
+ Partials 83 82 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
What
Cache.Runkeyed its compiled-program cache (c.scripts) by filename only, butSourceProgramOptionsresolves each identifier as predeclared vs global against the current predeclared name set (dict.Has). So running the same file under a different set of globals reused a stale program:Run("p.star", {"x":1})compilesout = xwithxpredeclared → ok;Run("p.star", nil)reuses that program and fails at run time withinternal error: predeclared variable x is uninitialized(or, in the reverse direction, silently reuses a program resolved under the wrong name set).This is the starlight twin of the issue already fixed in starlet's
exec.gogetCacheKey.Fix
Fold the sorted predeclared name set into the cache key (
scriptCacheKey); the dialect is fixed (dialectOptions), sofilename + namessuffices.Forgetnow drops every key belonging to a filename (a file may have several entries, one per distinct global-name set).Test-first
TestCachePredeclaredNameSetreproduces the stale-program internal error before the fix and pins the corrected behavior (cleanundefined: xresolve error; same-name-set runs still hit the cache).Verification
go test -race -count=2 ./...,go vet,gofmt -lclean, and the Dockergolang:1.19race run all green.Backlog: SLT-17