Skip to content

fix: stabilize capped rechecks in dependency order#4190

Open
teerthsharma wants to merge 4 commits into
facebook:mainfrom
teerthsharma:fix-capped-recheck-propagation-panic
Open

fix: stabilize capped rechecks in dependency order#4190
teerthsharma wants to merge 4 commits into
facebook:mainfrom
teerthsharma:fix-capped-recheck-propagation-panic

Conversation

@teerthsharma

Copy link
Copy Markdown
Contributor

Summary

  • preserve the live export-change frontier at the epoch cap
  • restrict the importer-to-dependency graph to its affected reverse-dependency closure
  • condense the closure into SCCs with the existing Tarjan implementation and stabilize them dependency-first

Why

The old fallback reread TransactionData.changed after the epoch loop had already drained it, so it invalidated an empty set and could commit with a newly pending change.

For the affected graph G[A], the condensation G[A] / ~ is a DAG. With edges importer -> dependency, Tarjan's SCC order is dependency-first, so every dependency component reaches a fixed point before its importers run. Pending work for later components is preserved, and no residual transaction state is cleared.

No new dependency is added; the normal fine-grained path is unchanged.

Verification

  • cargo test -p pyrefly --lib test_deep_scc_chain_stabilizes_after_epoch_cap -- --nocapture
  • cargo test -p pyrefly --lib test::incremental (75 passed)
  • cargo check -p pyrefly
  • python3 test.py --no-test --no-tensor-shapes --no-conformance --no-jsonschema

Built on #4180. Fixes #4171.

A finite chain of cyclic module pairs can consume the incremental epoch budget and reach commit with recorded export changes still pending.

Keep the current failure as an expected-panic regression so a separate scheduler fix can remove the annotation and verify propagation reaches the final importer.
The epoch cap discarded its live export-change frontier, so its final invalidation could leave a new change pending at commit. Preserve that frontier, condense its affected reverse-dependency graph into SCCs, and stabilize each component dependency-first before committing.
@teerthsharma
teerthsharma force-pushed the fix-capped-recheck-propagation-panic branch from 550ac4d to 7ffb5a9 Compare July 17, 2026 20:21
@github-actions github-actions Bot added size/m and removed size/m labels Jul 17, 2026
@teerthsharma
teerthsharma marked this pull request as ready for review July 17, 2026 20:25
@github-actions github-actions Bot added size/s and removed size/m labels Jul 18, 2026
@github-actions

This comment has been minimized.

Merging main restored the reproducer's expected-panic marker after the scheduler fix. Remove it again so CI verifies the transaction converges and reaches the final importer error.
@github-actions github-actions Bot added size/s and removed size/s labels Jul 18, 2026
@github-actions

Copy link
Copy Markdown

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LSP recheck can panic after an export change propagates through a deep dependency chain

1 participant