[ty] Fix some configuration panics in the LSP#22040
Merged
Conversation
This introduces a safe_mode to our settings type that tells our initialization code to more aggressively believe Settings Failure Is Not An Error. This allows the full settings initialization logic to be used as much as possible for the default database while trying to keep any discarded results as narrow as possible for graceful degradation. Finally, Python has SFINAE.
Gankra
commented
Dec 17, 2025
| insta::assert_json_snapshot!(show_message_params, @r#" | ||
| { | ||
| "type": 1, | ||
| "message": "Failed to load project rooted at <temp_dir>/project. Please refer to the logs for more details." |
Contributor
Author
There was a problem hiding this comment.
Note this message is not "we crash and die" but rather "we are using the default fallback database that all python files outside the workspace have" (so hopefully graceful degradation, but enough of a degradation we probably want the user to look into it and fix it).
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
|
Contributor
Author
|
I can't for the life of me fix this bloody windows path escaping, please someone save me. |
Contributor
Author
|
I guess I can just, not snapshot the message. |
CodSpeed Performance ReportMerging #22040 will not alter performanceComparing Summary
|
Contributor
Author
|
That codspeed result has to be a false positive |
MichaReiser
reviewed
Dec 18, 2025
Comment on lines
+27
to
+29
| /// Environment variable overrides. If a key is present here, it takes precedence | ||
| /// over the inner system's environment variables. | ||
| env_overrides: Arc<Mutex<FxHashMap<String, Option<String>>>>, |
Member
There was a problem hiding this comment.
I'm inclined to add this InMemorySystem instead.
Contributor
Author
There was a problem hiding this comment.
The e2e tests don't use InMemorySystem, alas.
Comment on lines
+97
to
+103
|
|
||
| /// Settings Failure Is Not An Error. | ||
| /// | ||
| /// This is used by the default database, which we are incentivized to make infallible, | ||
| /// while still trying to "do our best" to set things up properly where we can. | ||
| #[serde(default, skip)] | ||
| pub safe_mode: bool, |
Member
There was a problem hiding this comment.
The idea of Options is that they directly resemble the configuration file/CLI options. It shouldn't contain any program state or inferred values. I think I would either make safe_mode an argument (or provide methods like ProjectDatabase::default(metadata_, system) or/and add it to ProjectMetadata (we already store some derived CLI state on ProjectMetadata)
Contributor
Author
There was a problem hiding this comment.
Pushed up a shot at this.
MichaReiser
approved these changes
Dec 18, 2025
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
3 participants
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 is a revival of #21047 now that we have a reproducer again.
ProgramSettingsof default database are invalid ty#859Test Plan
e2e test from @zanieb