Repository navigation
feat(task): include dependency artifacts in cache keys - #11340
Conversation
📝 WalkthroughWalkthroughTask dependency tracking now carries stable artifact identities and structured completion outcomes. Artifact cache keys include dependency keys, enabling dependent restoration and conservative execution when identities are unavailable. Scheduling, documentation, and end-to-end coverage are updated. ChangesTask cache dependency parity
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TaskScheduler
participant Deps
participant TaskExecutor
participant TaskArtifactCache
TaskScheduler->>Deps: Compute dependency state
TaskScheduler->>TaskExecutor: Run task with completion state
TaskExecutor->>TaskArtifactCache: Build key with dependency identities
TaskArtifactCache-->>TaskExecutor: Return cache result and identity
TaskExecutor-->>TaskScheduler: Return work and cache-key outcome
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR incorporates dependency artifact identities into task cache keys and propagates completion state through nested task graphs.
Confidence Score: 5/5The PR appears safe to merge. The previously reported stale nested completion-state issue is resolved: every successful injected graph returns its completed task state, that state is merged before the next sequential entry, and the subsequent graph prunes tasks using the same task identity. Important Files Changed
Reviews (2): Last reviewed commit: "fix(task): propagate dependency cache st..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/task/deps.rs (1)
344-363: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding unit tests for
dependency_state/new_prunedcache-identity propagation.This is the core of the new caching contract (three interacting signals:
cache_keys,any_did_work,any_unkeyed_did_work), but the existing test module only covers cycle-detection. A few focused unit tests (e.g. a chain A→B→C verifyingany_unkeyed_did_workwhen B has no key, and cache-key propagation survivingnew_pruned) would guard against regressions in this fairly intricate logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/task/deps.rs` around lines 344 - 363, Add focused unit tests covering dependency_state’s three signals: cache_keys, any_did_work, and any_unkeyed_did_work. Include an A→B→C dependency-chain case where B performed work without a cache key, and a new_pruned case verifying cache-key propagation remains intact; place them alongside the existing cycle-detection tests and use the established test helpers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/task/deps.rs`:
- Around line 344-363: Update dependency_state() to include depends_post
relationships from post_dep_parents, folding cache keys and execution flags from
each post-dependency parent into TaskDependencyState alongside dep_edges. Ensure
artifact restoration or execution gating also requires those post-depend parents
to have executed or restored successfully when the post-task consumes their
outputs.
In `@TASK_CACHE_PARITY.md`:
- Line 81: Update the checklist item “Allow dependents to restore after
dependencies execute or restore” in TASK_CACHE_PARITY.md to qualify restoration
on dependencies publishing stable artifact keys; explicitly preserve that
unkeyed dependency work forces dependents to execute.
---
Nitpick comments:
In `@src/task/deps.rs`:
- Around line 344-363: Add focused unit tests covering dependency_state’s three
signals: cache_keys, any_did_work, and any_unkeyed_did_work. Include an A→B→C
dependency-chain case where B performed work without a cache key, and a
new_pruned case verifying cache-key propagation remains intact; place them
alongside the existing cycle-detection tests and use the established test
helpers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 69e4620f-e0e7-4854-8fc8-cea325c0115f
📒 Files selected for processing (8)
TASK_CACHE_PARITY.mddocs/tasks/task-configuration.mde2e/tasks/test_task_artifact_cache_dependenciessrc/cli/run.rssrc/task/deps.rssrc/task/mod.rssrc/task/task_cache.rssrc/task/task_executor.rs
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/tasks/test_task_artifact_cache_dependencies (1)
81-87: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert every restored artifact, not only the log and run counters.
These checks can pass while one deleted output remains missing:
app-distcontains copiedlibcontent, so it does not provelib-distwas restored, and the post-dependency case never checks either output before the later source change recreates them. Add explicit content assertions immediately after each restore.Proposed assertions
+assert "cat app-dist/result.txt" $'lib-one\napp' ... +assert "cat lib-dist/result.txt" "lib-one" ... +assert "cat parent-dist/result.txt" "parent-one" +assert "cat after-dist/result.txt" $'parent-one\nafter'Also applies to: 88-95, 104-112, 129-132
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/tasks/test_task_artifact_cache_dependencies` around lines 81 - 87, Expand the restore checks in the task artifact cache dependency test to assert the expected contents of every restored artifact, not just log and run counters. Add explicit content assertions immediately after each restore, including both lib-dist and app-dist in the initial and post-dependency restore cases, and preserve the existing checks around later source changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@e2e/tasks/test_task_artifact_cache_dependencies`:
- Around line 81-87: Expand the restore checks in the task artifact cache
dependency test to assert the expected contents of every restored artifact, not
just log and run counters. Add explicit content assertions immediately after
each restore, including both lib-dist and app-dist in the initial and
post-dependency restore cases, and preserve the existing checks around later
source changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: de6e0b01-e42f-4772-8253-bc9b15be4ad6
📒 Files selected for processing (5)
TASK_CACHE_PARITY.mde2e/tasks/test_task_artifact_cache_dependenciese2e/tasks/test_task_delegation_dedupsrc/task/deps.rssrc/task/task_executor.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- TASK_CACHE_PARITY.md
- src/task/deps.rs
- src/task/task_executor.rs
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.7.14 x -- echo |
16.9 ± 0.8 | 15.7 | 20.6 | 1.00 |
mise x -- echo |
18.6 ± 1.4 | 16.7 | 33.5 | 1.10 ± 0.09 |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.7.14 env |
16.1 ± 0.7 | 14.7 | 19.5 | 1.00 |
mise env |
17.4 ± 0.8 | 16.0 | 21.5 | 1.08 ± 0.07 |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.7.14 hook-env |
16.8 ± 0.8 | 15.6 | 19.8 | 1.00 |
mise hook-env |
18.2 ± 1.0 | 16.8 | 24.1 | 1.08 ± 0.07 |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.7.14 ls |
14.6 ± 0.7 | 13.4 | 17.4 | 1.00 |
mise ls |
15.9 ± 0.9 | 14.5 | 20.5 | 1.09 ± 0.08 |
xtasks/test/perf
| Command | mise-2026.7.14 | mise | Variance |
|---|---|---|---|
| install (cached) | 150ms | 165ms | -9% |
| ls (cached) | 55ms | 59ms | -6% |
| bin-paths (cached) | 58ms | 61ms | -4% |
| task-ls (cached) | 121ms | 125ms | -3% |
Summary
This is the next experimental task-cache follow-up to #11328. It makes downstream artifact identities transitively reflect the dependency graph, closing the key prerequisite for changed-workspace execution parity.
User impact
Cacheable task graphs can now reuse downstream artifacts when their dependencies resolve to the same artifact identities. Changing an upstream task's inputs changes its artifact key and therefore invalidates the matching downstream cache key. Uncached or failed-to-store upstream work remains conservative and forces downstream execution.
Validation
mise run lint-fixcargo fmt --all -- --checkcargo check --all-featuresmise run test:e2e test_task_artifact_cache test_task_dep_invalidates_sources test_task_delegation_dedupNote
Medium Risk
Changes core task scheduling, cache key hashing, and nested graph state propagation; incorrect key or dependency logic could cause stale restores or missed invalidation, though conservative unkeyed-dependency handling and new e2e tests mitigate this.
Overview
Experimental task artifact caching now folds direct dependency artifact keys into each task’s cache key, so downstream artifacts stay aligned with upstream inputs across execute, skip, and restore paths.
The scheduler tracks
TaskCompletionState(completed tasks, whether they did work, and published cache keys) andTaskDependencyStateper task. Dependents can restore from cache when dependencies expose stable keys; if a dependency ran without a key (e.g. cache disabled), dependents still execute conservatively.depends_postparents contribute keys the same way as regular dependencies. Nestedrun = [{ task = ... }]graphs merge completion state so delegated tasks do not rerun shared work.Docs and the parity tracker describe the new key material and behavior; new e2e coverage exercises lib→app restores, invalidation on upstream change, uncached deps, and post-deps.
Reviewed by Cursor Bugbot for commit 77dc5f9. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Bug Fixes
New Features
Documentation
Tests