Bugfix: Scope macro digest cache to each hash invocation

cli/src/main/kotlin/com/bazel_diff/hash/BuildGraphHasher.kt

@@ -44,6 +44,7 @@ class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent {
       modifiedFilepaths: Set<Path> = emptySet(),
       timings: HasherPhaseTimings? = null
   ): Map<String, TargetHash> {
+    val hashInvocationContext = HashInvocationContext()
     val (sourceDigests, allTargets) =
         runBlocking {
           val queryStartNanos = System.nanoTime()
@@ -85,7 +86,8 @@ class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent {
             sourceDigests,
             allTargets,
             ignoredAttrs,
-            modifiedFilepaths)
+            modifiedFilepaths,
+            hashInvocationContext)
     timings?.targetHashMillis = (System.nanoTime() - targetHashStartNanos) / 1_000_000
     return result
   }
@@ -132,7 +134,8 @@ class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent {
       sourceDigests: ConcurrentMap<String, ByteArray>,
       allTargets: List<BazelTarget>,
       ignoredAttrs: Set<String>,
-      modifiedFilepaths: Set<Path>
+      modifiedFilepaths: Set<Path>,
+      hashInvocationContext: HashInvocationContext
   ): Map<String, TargetHash> {
     val ruleHashes: ConcurrentMap<String, TargetDigest> = ConcurrentHashMap()
     val targetToRule: MutableMap<String, BazelRule> = HashMap()
@@ -150,7 +153,8 @@ class BuildGraphHasher(private val bazelClient: BazelClient) : KoinComponent {
                   seedHash,
                   packageBzlSeeds,
                   ignoredAttrs,
-                  modifiedFilepaths)
+                  modifiedFilepaths,
+                  hashInvocationContext)
           Pair(
               target.name,
               TargetHash(

cli/src/main/kotlin/com/bazel_diff/hash/HashInvocationContext.kt

@@ -0,0 +1,23 @@
+package com.bazel_diff.hash
+
+import java.util.concurrent.ConcurrentHashMap
+
+/**
+ * Mutable state that may be shared while one build graph is hashed.
+ *
+ * Hashers are application singletons and may outlive a workspace checkout. Revision-derived data
+ * must therefore live here instead of on a hasher. The concurrent map preserves digest
+ * deduplication while targets are hashed in parallel.
+ */
+class HashInvocationContext {
+  private val bzlDigests = ConcurrentHashMap<String, ByteArray>()
+
+  fun bzlDigest(path: String, calculate: () -> ByteArray?): ByteArray? {
+    val digest = bzlDigests.computeIfAbsent(path) { calculate() ?: MISSING_DIGEST }
+    return digest.takeUnless { it === MISSING_DIGEST }
+  }
+
+  private companion object {
+    val MISSING_DIGEST = ByteArray(0)
+  }
+}

cli/src/main/kotlin/com/bazel_diff/hash/RuleHasher.kt

@@ -6,7 +6,6 @@ import com.bazel_diff.bazel.decodeConfiguredRuleInputLabel
 import com.bazel_diff.log.Logger
 import com.google.common.annotations.VisibleForTesting
 import java.nio.file.Path
-import java.util.concurrent.ConcurrentHashMap
 import java.util.concurrent.ConcurrentMap
 import org.koin.core.component.KoinComponent
 import org.koin.core.component.inject
@@ -21,10 +20,6 @@ class RuleHasher(
   private val logger: Logger by inject()
   private val sourceFileHasher: SourceFileHasher by inject()
 
-  // main-repo `.bzl` path -> content digest; EMPTY marks a non-main-repo/missing file (skip).
-  private val bzlDigestCache = ConcurrentHashMap<String, ByteArray>()
-  private val EMPTY = ByteArray(0)
-
   /**
    * Per-rule `.bzl` seed: digests of the main-repo `.bzl` files in this rule's macro instantiation
    * stack, so a macro edit re-hashes only the rules that macro produced (issue #365).
@@ -33,7 +28,11 @@ class RuleHasher(
    * off) so the caller falls back to the package seed; a macro-less rule returns a stable constant,
    * so the caller does NOT fall back.
    */
-  private fun ruleBzlSeed(rule: BazelRule, modifiedFilepaths: Set<Path>): ByteArray? {
+  private fun ruleBzlSeed(
+      rule: BazelRule,
+      modifiedFilepaths: Set<Path>,
+      hashInvocationContext: HashInvocationContext
+  ): ByteArray? {
     val stack = rule.instantiationStack
     if (stack.isEmpty()) return null
     val bzlPaths = sortedSetOf<String>()
@@ -49,11 +48,11 @@ class RuleHasher(
     return sha256 {
       for (path in bzlPaths) {
         val digest =
-            bzlDigestCache.computeIfAbsent(path) {
+            hashInvocationContext.bzlDigest(path) {
               sourceFileHasher.softDigest(
-                  BazelSourceFileTarget("//$it", ByteArray(0)), modifiedFilepaths) ?: EMPTY
+                  BazelSourceFileTarget("//$path", ByteArray(0)), modifiedFilepaths)
             }
-        if (digest.isNotEmpty()) {
+        if (digest != null) {
           safePutBytes(path.toByteArray())
           safePutBytes(digest)
         }
@@ -84,7 +83,8 @@ class RuleHasher(
       packageBzlSeeds: Map<String, ByteArray>,
       depPath: LinkedHashSet<String>?,
       ignoredAttrs: Set<String>,
-      modifiedFilepaths: Set<Path>
+      modifiedFilepaths: Set<Path>,
+      hashInvocationContext: HashInvocationContext
   ): TargetDigest {
     val depPathClone = if (depPath != null) LinkedHashSet(depPath) else LinkedHashSet()
     if (depPathClone.contains(rule.name)) {
@@ -111,7 +111,8 @@ class RuleHasher(
           // Per-rule macro seed (see ruleBzlSeed), else the package-wide fallback. Each rule
           // resolves its own seed, so this is stable under the memoized recursion below (#365).
           putDirectBytes(
-              ruleBzlSeed(rule, modifiedFilepaths) ?: packageBzlSeeds[labelToPackage(rule.name)])
+              ruleBzlSeed(rule, modifiedFilepaths, hashInvocationContext)
+                  ?: packageBzlSeeds[labelToPackage(rule.name)])
           // Mixed into the *direct* digest (not transitively) so the tagged target is classified
           // as DIRECT-impacted for distance metrics; it still bubbles into the overall digest, so
           // any rdeps are conservatively re-hashed too.
@@ -144,7 +145,8 @@ class RuleHasher(
                         packageBzlSeeds,
                         depPathClone,
                         ignoredAttrs,
-                        modifiedFilepaths)
+                        modifiedFilepaths,
+                        hashInvocationContext)
                 putTransitiveBytes(inputLabel, ruleInputHash.overallDigest)
               }
               else -> {
@@ -192,7 +194,8 @@ class RuleHasher(
                       packageBzlSeeds,
                       depPathClone,
                       ignoredAttrs,
-                      modifiedFilepaths)
+                      modifiedFilepaths,
+                      hashInvocationContext)
               putTransitiveBytes(packageGroupLabel, packageGroupHash.overallDigest)
             }
           }

cli/src/main/kotlin/com/bazel_diff/hash/TargetHasher.kt

@@ -18,7 +18,8 @@ class TargetHasher : KoinComponent {
       seedHash: ByteArray?,
       packageBzlSeeds: Map<String, ByteArray>,
       ignoredAttrs: Set<String>,
-      modifiedFilepaths: Set<Path>
+      modifiedFilepaths: Set<Path>,
+      hashInvocationContext: HashInvocationContext
   ): TargetDigest {
     return when (target) {
       is BazelTarget.GeneratedFile -> {
@@ -41,7 +42,8 @@ class TargetHasher : KoinComponent {
                   packageBzlSeeds,
                   depPath = null,
                   ignoredAttrs,
-                  modifiedFilepaths)
+                  modifiedFilepaths,
+                  hashInvocationContext)
         }
 
         // Add the generating rule name as a dep of the generated file.
@@ -58,7 +60,8 @@ class TargetHasher : KoinComponent {
             packageBzlSeeds,
             depPath = null,
             ignoredAttrs,
-            modifiedFilepaths)
+            modifiedFilepaths,
+            hashInvocationContext)
       }
       is BazelTarget.SourceFile -> {
         // Mix in the `.bzl` seed for this source file's own package only, so a macro edit in

cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt

@@ -1,6 +1,7 @@
 package com.bazel_diff.e2e
 
 import assertk.assertThat
+import assertk.assertions.contains
 import assertk.assertions.isEqualTo
 import com.bazel_diff.cli.BazelDiff
 import com.google.gson.Gson
@@ -166,26 +167,16 @@ class E2ETest {
   @Test
   fun testServeEndToEnd() {
     val workspace = copyTestWorkspace("distance_metrics")
-    fun git(vararg args: String): String {
-      val proc =
-          ProcessBuilder(listOf("git") + args)
-              .directory(workspace)
-              .redirectErrorStream(true)
-              .start()
-      val output = proc.inputStream.readBytes().decodeToString()
-      check(proc.waitFor() == 0) { "git ${args.joinToString(" ")} failed: $output" }
-      return output.trim()
-    }
-    git("init", "-q")
-    git("config", "user.email", "test@example.com")
-    git("config", "user.name", "test")
-    git("add", "-A")
-    git("commit", "-q", "-m", "base")
-    val fromSha = git("rev-parse", "HEAD")
+    git(workspace, "init", "-q")
+    git(workspace, "config", "user.email", "test@example.com")
+    git(workspace, "config", "user.name", "test")
+    git(workspace, "add", "-A")
+    git(workspace, "commit", "-q", "-m", "base")
+    val fromSha = git(workspace, "rev-parse", "HEAD")
     File(workspace, "lib.sh").writeText("echo changed\n")
-    git("add", "-A")
-    git("commit", "-q", "-m", "change lib.sh")
-    val toSha = git("rev-parse", "HEAD")
+    git(workspace, "add", "-A")
+    git(workspace, "commit", "-q", "-m", "change lib.sh")
+    val toSha = git(workspace, "rev-parse", "HEAD")
 
     val cacheDir = temp.newFolder()
     val port = java.net.ServerSocket(0).use { it.localPort }
@@ -247,6 +238,66 @@ class E2ETest {
     }
   }
 
+  @Test
+  fun testServeDoesNotReuseMacroDigestAcrossRevisions() {
+    val workspace = copyTestWorkspace("serve_bzl_cache")
+    git(workspace, "init", "-q")
+    git(workspace, "config", "user.email", "test@example.com")
+    git(workspace, "config", "user.name", "test")
+    git(workspace, "add", "-A")
+    git(workspace, "commit", "-q", "-m", "base")
+    val fromSha = git(workspace, "rev-parse", "HEAD")
+
+    File(workspace, "defs.bzl").appendText("\n# second revision\n")
+    git(workspace, "add", "-A")
+    git(workspace, "commit", "-q", "-m", "change macro source")
+    val toSha = git(workspace, "rev-parse", "HEAD")
+
+    val cacheDir = temp.newFolder()
+    val port = java.net.ServerSocket(0).use { it.localPort }
+    val serveThread =
+        Thread {
+              CommandLine(BazelDiff())
+                  .execute(
+                      "serve",
+                      "-w",
+                      workspace.absolutePath,
+                      "-b",
+                      "bazel",
+                      "--cacheDir",
+                      cacheDir.absolutePath,
+                      "--port",
+                      port.toString(),
+                      "--no-initial-fetch")
+            }
+            .apply {
+              isDaemon = true
+              start()
+            }
+
+    try {
+      awaitServeHealthy(port)
+      val (code, body) =
+          httpGetServe("http://localhost:$port/impacted_targets?from=$fromSha&to=$toSha")
+      assertThat(code).isEqualTo(200)
+      val parsed: Map<String, Any> =
+          Gson().fromJson(body, object : TypeToken<Map<String, Any>>() {}.type)
+      @Suppress("UNCHECKED_CAST") val impacted = parsed["impactedTargets"] as List<String>
+      assertThat(impacted.map { it.removePrefix("@@") }).contains("//:generated")
+    } finally {
+      serveThread.interrupt()
+      serveThread.join(10_000)
+    }
+  }
+
+  private fun git(workspace: File, vararg args: String): String {
+    val proc =
+        ProcessBuilder(listOf("git") + args).directory(workspace).redirectErrorStream(true).start()
+    val output = proc.inputStream.readBytes().decodeToString()
+    check(proc.waitFor() == 0) { "git ${args.joinToString(" ")} failed: $output" }
+    return output.trim()
+  }
+
   /** Polls `/health` until it returns 200, up to ~30s, failing the test otherwise. */
   private fun awaitServeHealthy(port: Int) {
     repeat(60) {

cli/src/test/kotlin/com/bazel_diff/hash/BuildGraphHasherTest.kt

@@ -60,6 +60,8 @@ class BuildGraphHasherTest : KoinTest {
           add(createRuleTarget(name = "rule2", inputs = ArrayList(), digest = "rule2Digest"))
         }
     Mockito.reset(bazelClientMock)
+    fakeSourceFileHasher.softDigestValue = "fake-soft-digest".toByteArray()
+    fakeSourceFileHasher.softDigestCalls.set(0)
   }
 
   @Test
@@ -344,16 +346,61 @@ class BuildGraphHasherTest : KoinTest {
             "rule3" to listOf("rule0"), "rule0" to listOf("rule1"), "rule1" to emptyList<String>())
   }
 
+  @Test
+  fun testMacroDigestIsScopedToOneHashInvocation() = runBlocking {
+    val target =
+        createRuleTarget(
+            name = "rule1",
+            inputs = emptyList(),
+            digest = "rule1Digest",
+            instantiationStack = listOf("defs/macro.bzl:1:1: macro"))
+    whenever(bazelClientMock.queryAllTargets()).thenReturn(listOf(target))
+
+    fakeSourceFileHasher.softDigestValue = "v1".toByteArray()
+    val first = hasher.hashAllBazelTargetsAndSourcefiles()
+    fakeSourceFileHasher.softDigestValue = "v2".toByteArray()
+    val second = hasher.hashAllBazelTargetsAndSourcefiles()
+    fakeSourceFileHasher.softDigestValue = "v1".toByteArray()
+    val third = hasher.hashAllBazelTargetsAndSourcefiles()
+
+    assertThat(second["rule1"]).isNotEqualTo(first["rule1"])
+    assertThat(third["rule1"]).isEqualTo(first["rule1"])
+  }
+
+  @Test
+  fun testMacroDigestIsReusedWithinHashInvocation() = runBlocking {
+    val instantiationStack = listOf("defs/macro.bzl:1:1: macro")
+    val first =
+        createRuleTarget(
+            name = "rule1",
+            inputs = emptyList(),
+            digest = "rule1Digest",
+            instantiationStack = instantiationStack)
+    val second =
+        createRuleTarget(
+            name = "rule2",
+            inputs = emptyList(),
+            digest = "rule2Digest",
+            instantiationStack = instantiationStack)
+    whenever(bazelClientMock.queryAllTargets()).thenReturn(listOf(first, second))
+
+    hasher.hashAllBazelTargetsAndSourcefiles()
+
+    assertThat(fakeSourceFileHasher.softDigestCalls.get()).isEqualTo(1)
+  }
+
   private fun createRuleTarget(
       name: String,
       inputs: List<String>,
-      digest: String
+      digest: String,
+      instantiationStack: List<String> = emptyList()
   ): BazelTarget.Rule {
     val target = mock<BazelTarget.Rule>()
     val rule = mock<BazelRule>()
     whenever(rule.name).thenReturn(name)
     whenever(rule.ruleInputList(false, emptySet())).thenReturn(inputs)
     whenever(rule.digest(emptySet())).thenReturn(digest.toByteArray())
+    whenever(rule.instantiationStack).thenReturn(instantiationStack)
     whenever(target.rule).thenReturn(rule)
     whenever(target.name).thenReturn(name)
     return target

cli/src/test/kotlin/com/bazel_diff/hash/FakeSourceFileHasher.kt

@@ -2,9 +2,12 @@ package com.bazel_diff.hash
 
 import com.bazel_diff.bazel.BazelSourceFileTarget
 import java.nio.file.Path
+import java.util.concurrent.atomic.AtomicInteger
 
 class FakeSourceFileHasher : SourceFileHasher {
   var fakeDigests: MutableMap<String, ByteArray> = mutableMapOf()
+  var softDigestValue: ByteArray? = "fake-soft-digest".toByteArray()
+  val softDigestCalls = AtomicInteger()
 
   override fun digest(
       sourceFileTarget: BazelSourceFileTarget,
@@ -18,7 +21,8 @@ class FakeSourceFileHasher : SourceFileHasher {
       sourceFileTarget: BazelSourceFileTarget,
       modifiedFilepaths: Set<Path>
   ): ByteArray? {
-    return "fake-soft-digest".toByteArray()
+    softDigestCalls.incrementAndGet()
+    return softDigestValue
   }
 
   fun add(name: String, digest: ByteArray) {

cli/src/test/kotlin/com/bazel_diff/hash/RuleHasherAlwaysAffectedTagsTest.kt

@@ -58,7 +58,8 @@ class RuleHasherAlwaysAffectedTagsTest : KoinTest {
         emptyMap(),
         null,
         emptySet(),
-        emptySet())
+        emptySet(),
+        HashInvocationContext())
   }
 
   @Test

cli/src/test/resources/workspaces/serve_bzl_cache/BUILD

@@ -0,0 +1,3 @@
+load(":defs.bzl", "make_generated")
+
+make_generated(name = "generated")