Skip to content

Include dialect in module cache keys#516

Draft
aqk wants to merge 2 commits into
aqk/fix-module-dialect-cse-dominancefrom
aqk/fix-module-cache-cse-dominance
Draft

Include dialect in module cache keys#516
aqk wants to merge 2 commits into
aqk/fix-module-dialect-cse-dominancefrom
aqk/fix-module-cache-cse-dominance

Conversation

@aqk

@aqk aqk commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add explicit dialect material, including cse_dominance, to module cache keys.
  • Update cache-key tests and module cache fixtures to use the option-aware cache key helper.

Test plan

  • cargo test module_cache
  • cargo test diskcache

Note

Low Risk
Compiler disk-cache keying only; intentional cache invalidation improves correctness without touching runtime auth or data paths.

Overview
Module export disk cache keys now incorporate compiler dialect settings, not just include fingerprints, so cached hex cannot be reused across incompatible compiler configurations.

cache_key takes CompilerOpts, builds module-cache-v2 key material from stepping, strict, int_fix, extra_numeric_constants, and cse_dominance, then appends include fingerprints before hashing. Lookups and writes already had opts; they now feed that into the key. The stable empty-include test expectation changes accordingly, and a new unit test asserts different keys when only cse_dominance toggles. Integration tests resolve .chialisp/<key>/ via the opts-aware module_cache_key_hex helper.

Reviewed by Cursor Bugbot for commit 9c21fd1. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9c21fd1. Configure here.

Comment thread src/compiler/diskcache.rs
key_material.extend_from_slice(&include.fingerprint);
}
hex::encode(sha256tree_from_atom(&include_fingerprints))
hex::encode(sha256tree_from_atom(&key_material))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cache key dialect not normalized

Medium Severity

cache_key hashes opts.dialect() as passed into try_from_cache, but module compilation rewrites dialect (int_fix, stepping floor, cse_dominance) before set_cache_element. Lookup and store therefore use different key material, so incremental module disk cache hits fail even when entries exist.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9c21fd1. Configure here.

@aqk aqk marked this pull request as draft July 7, 2026 00:33
aqk added 2 commits July 6, 2026 17:38
Prevent stale module artifacts from bypassing dialect-gated optimizer fixes by keying cached outputs on explicit dialect semantics.
Use the same effective module dialect for cache reads and writes so dialect-aware cache keys still hit valid module artifacts.
@aqk aqk force-pushed the aqk/fix-module-cache-cse-dominance branch from 9c21fd1 to a8f575a Compare July 7, 2026 00:39
@aqk aqk changed the base branch from 20260515-cse-dominance-trick to aqk/fix-module-dialect-cse-dominance July 7, 2026 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant