Skip to content

Treat impl-refresh hash misses as stale#2674

Merged
plajjan merged 2 commits intomainfrom
stale-impl
Mar 5, 2026
Merged

Treat impl-refresh hash misses as stale#2674
plajjan merged 2 commits intomainfrom
stale-impl

Conversation

@plajjan
Copy link
Contributor

@plajjan plajjan commented Mar 3, 2026

A cached TyTask can enter runImplRefresh when only impl dependency hashes change. In that path we load the typed module from .ty and recompute impl deps/hashes against current dependency name-hash maps.

If the cached .ty is internally stale or inconsistent, mkEnv/splitDeps/ hash resolution can fail and surface internal failures such as NoItem or hash-missing diagnostics, even though this is a recoverable stale-cache situation.

This change makes runImplRefresh fall back to runFront when impl-refresh cannot resolve dependency hashes reliably. This aligns impl-refresh behavior with stale-check handling where unresolved dep hashes trigger refresh rather than hard internal errors.

Add an incremental compiler test that forces a stale/inconsistent .ty
path through impl-only dependency changes.

The test rewrites main.ty to keep a stale typed-module dependency
while updating header fields enough to stay on the TyTask/impl-
refresh route.

Without the impl-refresh fallback fix this triggers an internal
failure (NoItem in this repro).
@chatgpt-codex-connector
Copy link

💡 Codex Review

handleImplRefreshException :: SomeException -> IO (TaskKey, Either [Diagnostic String] FrontResult)
handleImplRefreshException _ = rerunFront

P1 Badge Re-throw async exceptions in impl-refresh handler

runImplRefresh now catches SomeException and unconditionally falls back to runFront, which also captures cancellation exceptions (AsyncCancelled/ThreadKilled) delivered by cancel in startCompile. In watch/LSP flows this can cause a canceled generation to keep compiling instead of terminating, potentially running concurrently with the next generation and emitting stale work; this handler should rethrow async exceptions and only treat non-async failures as stale-cache fallback cases.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

A cached TyTask can enter runImplRefresh when only impl dependency
hashes change. In that path we load the typed module from .ty and
recompute impl deps/hashes against current dependency name-hash maps.

If the cached .ty is internally stale or inconsistent, mkEnv/splitDeps/
hash resolution can fail and surface internal failures such as NoItem
or hash-missing diagnostics, even though this is a recoverable stale-
cache situation.

This change makes runImplRefresh fall back to runFront when impl-
refresh cannot resolve dependency hashes reliably. It wraps impl-
refresh execution in an exception fallback, treats mkEnv and dep-hash
resolution failures as stale-cache signals, and emits a verbose stale
message before rerunning front passes.

Async cancellation exceptions are rethrown instead of treated as stale-
cache failures. This keeps cancel semantics intact in watch and LSP
flows and prevents canceled generations from continuing stale work.

This aligns impl-refresh behavior with stale-check handling where
unresolved dep hashes trigger refresh rather than hard internal errors.
@plajjan plajjan merged commit ea66dcc into main Mar 5, 2026
41 of 44 checks passed
@plajjan plajjan deleted the stale-impl branch March 5, 2026 09:11
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.

1 participant