Skip to content

fix(cache): restart continuous cache streak after compute steps - #1114

Merged
DefTruth merged 3 commits into
vipshop:mainfrom
chengyaoxue64-lab:fix/continuous-cached-step-count
Oct 13, 2026
Merged

DefTruth merged 3 commits into
vipshop:mainfrom
chengyaoxue64-lab:fix/continuous-cached-step-count

Conversation

@chengyaoxue64-lab

Copy link
Copy Markdown
Contributor

Summary

  • restart the continuous cache streak at one after a non-adjacent cache hit
  • apply the same accounting to normal and separate-CFG branches
  • add regression coverage for both CFG execution orders

Problem

When max_continuous_cached_steps=1 blocks a cache hit, can_cache() resets the streak counter to zero. The next non-adjacent hit was appended without setting the counter back to one, so the following adjacent step was also allowed.

With four warmup steps and 19 denoising steps, the buggy sequence was [4, 6, 7, 9, 10, 12, 13, 15, 16, 18]; the limit should produce [4, 6, 8, 10, 12, 14, 16, 18].

Test plan

  • pytest tests/api/test_refresh_context.py -q (9 passed)
  • pre-commit run --files src/cache_dit/caching/cache_contexts/cache_context.py tests/api/test_refresh_context.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No unresolved blocking issues were identified.

Pull request overview

Fixes continuous cache streak accounting after compute steps, including separate-CFG execution.

Changes:

  • Correct streak resets for normal and CFG paths.
  • Add regression coverage for both CFG execution orders.
File summaries
File Description
tests/api/test_refresh_context.py Adds regression tests for cache limits and CFG modes.
src/cache_dit/caching/cache_contexts/cache_context.py Corrects cache streak tracking.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Forced warmup or computation-mask steps can leave stale streak counters and incorrectly reject the next cache opportunity.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

src/cache_dit/caching/cache_contexts/cache_context.py:263

  • The CFG streak has the same stale-counter problem: a CFG compute forced by an early can_cache() return (such as an interleaved warmup or dynamic computation-mask step) does not clear this value, so the next non-adjacent CFG cache candidate can be rejected before this branch gets a chance to restart the streak. Reset or derive the CFG streak from adjacency before enforcing the limit, and cover both CFG execution orders with an interleaved forced-compute case.
      else:
        self.cfg_continuous_cached_steps = 1
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/cache_dit/caching/cache_contexts/cache_context.py Outdated
Reset per-branch streaks when the previous step was not cached.
Remove redundant cap resets and simplify hit counting.

Cover interleaved warmup, dynamic computation masks, both CFG
orders, and repeated cache checks in regression tests.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The branch-aware accounting is consistent and comprehensively covered by targeted regression cases.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

@DefTruth DefTruth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for this fix!

@DefTruth
DefTruth merged commit 170e3ab into vipshop:main Oct 13, 2026
4 checks passed
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.

4 participants