[ty] Fix untracked reads in Salsa queries that can lead to backdating panics#24051
Merged
MichaReiser merged 1 commit intomainfrom Mar 20, 2026
Merged
[ty] Fix untracked reads in Salsa queries that can lead to backdating panics#24051MichaReiser merged 1 commit intomainfrom
MichaReiser merged 1 commit intomainfrom
Conversation
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 85.29%. The percentage of expected errors that received a diagnostic held steady at 78.13%. The number of fully passing files held steady at 64/132. |
Memory usage reportMemory usage unchanged ✅ |
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-await |
40 | 0 | 0 |
invalid-return-type |
1 | 0 | 0 |
| Total | 41 | 0 | 0 |
Changes in flaky projects detected. Raw diff output excludes flaky projects; see the HTML report for details.
MichaReiser
commented
Mar 19, 2026
|
|
||
| for change in changes { | ||
| tracing::trace!("Handle change: {:?}", change); | ||
| tracing::debug!("Handling file watcher change event: {:?}", change); |
Member
Author
There was a problem hiding this comment.
I was torn on whether promoting this to debug is a good idea, but debugging production file watcher issues is sort of impossible without knowing what "happened". But it has the downside of being somewhat verbose.
ibraheemdev
approved these changes
Mar 19, 2026
Member
ibraheemdev
left a comment
There was a problem hiding this comment.
Great job tracking this down!
This was referenced Mar 20, 2026
Merged
carljm
added a commit
that referenced
this pull request
Mar 25, 2026
* main: [`flake8-bandit`] Check tuple arguments for partial paths in `S607` (#24080) [ty] Update Salsa (#24081) Update Rust toolchain to 1.94 and MSRV to 1.92 (#24076) [ty] Move ruffen-docs formatting config to a `ruff.toml` config file (#24074) [ty] `reveal_type` diagnostics in unreachable code (#24070) [ty] Improve keyword argument narrowing for nested dictionaries (#24010) [ty] Preserve blank lines between comments and imports in add-import action (#24066) [ty] Add diagnostic hint for invalid assignments involving invariant generics (#24032) Clarify `extend-ignore` and `extend-select` settings documentation (#24064) [ty] Batch changes to watched paths (#24045) replace deprecated `std::f64::EPSILON` with `f64::EPSILON` (#24067) [ty] Fix untracked reads in Salsa queries that can lead to backdating panics (#24051) [ty] Unions/intersections of gradual types should be assignable to `Never` (#24056) Fix incorrect path for ty_python_semantic in fuzzer (#24052) Bump 0.15.7 (#24049) [ty] ecosystem-analyzer: Fail on newly panicking projects (#24043) Don't show noqa hover for non-Python documents (#24040)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes two cases that could lead to the panics reported in astral-sh/ty#1565.
After a change to an input (e.g., a file was modified), Salsa re-executes all queries that read that input or depend on any such query. To limit invalidation, Salsa can backdate a query. If a query computes to the same value as in the previous revision, Salsa considers it unchanged and skips re-executing dependent queries. This is called backdating a query. The reason it's called backdating is that Salsa tracks the revision in which a query was last changed; backdating sets the last-changed revision "back" to the revision from before the input change. During backdating, Salsa enforces that the revision is indeed going backwards. That is, the revision Salsa backdates to (or the last changed revision from the previous query run) must be smaller (older) than the last changed revision from the current run.
Under normal circumstances, this invariant should always hold because for a query result to change, the query has to read some input that was changed in the current revision (at least in a revision newer than the last run). This ensures that the last changed revision from the new run is never older than that from the previous run. However, this invariant can be violated if a query branches on a state that isn't tracked in Salsa. Salsa will rerun the query because one of its inputs changed, but the query branches on some untracked state before reading that input, so that the query's last changed revision might be earlier than when the input changed.
Reading an untracked state in a query is always a bug (unless you call
db.untracked_read), and it can lead to stale results. But I don't think it's an error severe enough to warrant Salsa panicking. I plan to PR a change to Salsa to panic only in debug builds and otherwise log a warning, notifying users about the untracked read.The change in this PR is to fix two cases where we ended up with reading untracked global state or failed to correctly invalidate all query dependencies:
Projectis a Salsa input, butdb.projectisn't a Salsa tracked field. Therefore, it's important that we never reassigndb.projectduring a DB's lifetime. This PR fixes the one place where we did replacedb.projectwith a newProjectFile::syncwhere we should. This can lead to stale results and potentially panic withinabsolute_desperate_search_pathsbecause the query branches on whether something is a file.Test Plan
I wrote three integration tests with Codex that all panicked before making the above changes. I decided not to commit them because they all feel a bit artificial and I didn't manage to come up with more real-world tests that reproduce the panic. All tests now pass without panicking.
Tests