feat: content-hash eval-base image tags to auto-rebuild on Dockerfile changes

[simonrosenberg]

Summary

• Include a 7-char SHA-256 content hash of the SDK Dockerfile in eval-base image tags (eval-base:{hash}-{instance_tag}) so that Dockerfile changes automatically invalidate cached images and trigger rebuilds
• Extract dockerfile_content_hash() as a public helper, compute it once per phase in the orchestrator, and pass the result through to ProcessPoolExecutor workers — base_image_tag() stays a pure string formatter
• content_hash is a required keyword-only arg — no silent fallback to file I/O
• Covers swebench, swebench-multimodal, and swtbench (all go through the same 3-phase pipeline via benchmarks/swebench/build_images.py)

Closes #593

Validation

Triggered real CI builds for django__django-11099 on the feature branch:

Run	Scenario	Base image	Result
#23869153028	Cold build (new tag format)	eval-base:245a238-sweb.eval.x86_64.django_1776_django-11099	Built and pushed
#23869164900	Warm (same Dockerfile)	eval-base:245a238-sweb.eval.x86_64.django_1776_django-11099	Skipped (already exists)

• Run 1 built the base image with content hash 245a238 in the tag
• Run 2 found the same tag in GHCR and skipped all 3 phases (builder, base, assembly)
• Both phases (1 and 2) produce matching tags — assembly correctly references the base image pushed in phase 1

Test plan

• All 22 test_phased_build.py tests pass (including new test_dockerfile_content_hash_format)
• Pre-commit hooks pass (ruff format, ruff lint, pycodestyle, pyright strict)
• CI validation: cold build succeeds and pushes content-hashed tag
• CI validation: warm build skips when Dockerfile unchanged

🤖 Generated with Claude Code

[all-hands-bot]

🟡 Acceptable - Solves the real problem (Dockerfile changes auto-invalidate images) but adds unnecessary complexity. Core logic is sound, tests are comprehensive, but the caching approach creates test pollution.

[all-hands-bot]

🟡 Acceptable - Solves the real problem (Dockerfile changes auto-invalidate cached images), but the design creates unnecessary complexity and I/O overhead. The PR description is misleading about caching.

[all-hands-bot]

🟠 Important: This reads and hashes the Dockerfile on every call to base_image_tag(). With 500+ images mentioned in the description, that's 500+ redundant file reads and hash computations.

Linus-style critique: Wrong data structure. You're treating a constant (Dockerfile hash) as a computed value. This should be calculated once at module load time:

Suggested change

	dockerfile = _get_sdk_dockerfile().read_text()
	content_hash = hashlib.sha256(dockerfile.encode()).hexdigest()[:7]
	return f"{image}:{content_hash}-{custom_tag}"
	# At module level (after _get_sdk_dockerfile definition):
	_DOCKERFILE_HASH = hashlib.sha256(
	_get_sdk_dockerfile().read_text().encode()
	).hexdigest()[:7]
	def base_image_tag(custom_tag: str, image: str = EVAL_BASE_IMAGE) -> str:
	"""Compute the full registry tag for a pre-built base image.
	The tag includes a short content hash of the SDK Dockerfile so that
	any change to the Dockerfile automatically invalidates cached images.
	"""
	return f"{image}:{_DOCKERFILE_HASH}-{custom_tag}"

This would:

• Eliminate 500+ file reads (compute once per process)
• Simplify all tests (no mocking needed - tests can just verify tag format)
• Make the hidden I/O explicit and obvious
• Follow the principle: "Data structures, not code"

Alternatively, if you prefer lazy evaluation, use @functools.cache on a helper function (Python 3.9+).

[all-hands-bot]

🟡 Suggestion: This mocking pattern repeats across 6+ tests. It's test pollution caused by the design choice to compute the hash on every call.

If the hash were a module-level constant, these tests wouldn't need any mocking - they'd just verify the tag format includes a 7-char hex prefix. Simpler tests are easier to maintain and less brittle.

[all-hands-bot]

🟢 Acceptable: This test validates real behavior (hash changes with content). Good test that exercises actual logic rather than just mocking.

However, note that it's testing a property that could be proven by inspection if the hash were a module constant.

[all-hands-bot]

🟡 Acceptable - Core logic is sound, but you're not testing what you document.

[TESTING GAPS]
The docstring promises "7-char SHA-256 content hash" but no test validates this format. If someone changes [:7] to [:8] or removes it, tests still pass.

VERDICT: ✅ Worth merging - but add format validation.

KEY INSIGHT: Tests validate behavior (hashes differ with content) but not the contract (7-char hex format).

[all-hands-bot]

🟠 Important: This test validates that different content produces different hashes, which is good. But the docstring in base_image_tag() claims "7-char content hash" and nothing validates that format.

Add an assertion to verify the documented guarantee:

import re

assert tag1 != tag2
# Validate the documented format: {image}:{7-hex-chars}-{custom_tag}
assert re.match(r'^.+:[0-9a-f]{7}-.+$', tag1), f"Tag format mismatch: {tag1}"

Without this, if someone changes [:7] to [:8] or [:6], your tests won't catch it. You're testing behavior, not the contract.

[all-hands-bot]

🟡 Acceptable - Solves the real problem (Dockerfile changes auto-invalidate cached images) but uses inefficient data structure. See inline comments for details.

[all-hands-bot]

🟠 Important - Testing Gap: The docstring promises "7-char SHA-256 content hash" but no test validates this format. The test correctly verifies that different content produces different hashes, but doesn't check the documented contract.

Add format validation:

import re

assert tag1 != tag2
assert re.match(r'^.+:[a-f0-9]{7}-', tag1), f"Expected 7-char hex prefix in tag: {tag1}"
assert re.match(r'^.+:[a-f0-9]{7}-', tag2), f"Expected 7-char hex prefix in tag: {tag2}"

[all-hands-bot]

🟡 Acceptable - Solves the real problem, but API has a footgun and tests don't validate the documented contract.

[all-hands-bot]

🔴 Needs improvement - API footgun allows malformed tags

[CRITICAL ISSUE]
The PR description claims content_hash is required, but build_base_image() has a default "" that creates broken tags like eval-base:-custom-tag. Either remove defaults everywhere or auto-compute the hash when missing.

VERDICT: ❌ Needs rework before merge

KEY INSIGHT: You've made it optional at the wrong layer - the helper functions allow broken state that the formatter would reject.

[all-hands-bot]

🟢 Good taste - Clean solution to a real problem.

This fixes the cache invalidation issue correctly: hash the Dockerfile, embed it in the tag, compute once per phase. The API is safe (required keyword-only arg), tests validate the contract, and the implementation has no special cases. The unresolved threads are bikeshedding about saving ~1ms with module-level constants - not worth the complexity.

Verdict: ✅ Ship it.