From 42345f134e389b32a6f03d3ec202f09f8a3de3a9 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 3 Jul 2026 15:26:58 +0200 Subject: [PATCH] Bump Salsa revision manually for non-Salsa updates --- crates/ark/src/lsp.rs | 1 - crates/ark/src/lsp/main_loop.rs | 32 ++++++++++++++-------- crates/ark/src/lsp/state.rs | 11 ++++++++ crates/ark/src/lsp/state_handlers.rs | 10 ++++--- crates/ark/src/lsp/tests/state_handlers.rs | 25 +++++++++++++++++ 5 files changed, 63 insertions(+), 16 deletions(-) diff --git a/crates/ark/src/lsp.rs b/crates/ark/src/lsp.rs index 90dd0f7aad..8aca53e830 100644 --- a/crates/ark/src/lsp.rs +++ b/crates/ark/src/lsp.rs @@ -69,6 +69,5 @@ pub(crate) use _log; pub(crate) use log_error; pub(crate) use log_info; pub(crate) use log_warn; -pub(crate) use main_loop::diagnostics_refresh_all; pub(crate) use main_loop::publish_diagnostics; pub(crate) use main_loop::spawn_blocking; diff --git a/crates/ark/src/lsp/main_loop.rs b/crates/ark/src/lsp/main_loop.rs index 81595eb54a..a3f6c01730 100644 --- a/crates/ark/src/lsp/main_loop.rs +++ b/crates/ark/src/lsp/main_loop.rs @@ -388,8 +388,9 @@ impl GlobalState { // resolved definitions), so any handler that writes to oak invalidates // them. Rather than have each write site remember to refresh, we watch // the oak revision across the whole tick: if a handler advanced it, - // refresh centrally. Config and console state live outside oak, so - // handlers that mutate those still refresh explicitly. + // refresh centrally. Config and console state live outside oak, so the + // handlers that mutate those advance the revision synthetically (see + // `WorldState::bump_revision`) to route through this same path. let old_revision = salsa::plumbing::current_revision(&self.world.db); match event { @@ -1031,15 +1032,12 @@ static DIAGNOSTICS_QUEUE: LazyLock) { while let Some(task) = rx.recv().await { let mut batch = vec![task]; @@ -1252,4 +1250,16 @@ mod tests { let after = salsa::plumbing::current_revision(&state.db); assert_ne!(before, after); } + + /// Console-scope and config changes live outside oak, so they lean on + /// `bump_revision` to advance the revision and trigger the same central + /// refresh. This pins that the synthetic write actually moves the revision. + #[test] + fn test_bump_revision_advances_revision() { + let mut state = WorldState::default(); + let before = salsa::plumbing::current_revision(&state.db); + state.bump_revision(); + let after = salsa::plumbing::current_revision(&state.db); + assert_ne!(before, after); + } } diff --git a/crates/ark/src/lsp/state.rs b/crates/ark/src/lsp/state.rs index 09a9a6ea74..da6e2f21b9 100644 --- a/crates/ark/src/lsp/state.rs +++ b/crates/ark/src/lsp/state.rs @@ -5,6 +5,7 @@ use anyhow::anyhow; use oak_db::File; use oak_db::OakDatabase; use oak_semantic::library::Library; +use salsa::Database; use url::Url; use crate::lsp::config::LspConfig; @@ -120,6 +121,16 @@ impl WorldState { workspace: Workspace::default(), } } + /// Advance the oak revision without changing any oak input. + /// + /// Currently used for state that lives on `WorldState` but not in the Oak + /// DB (e.g. console scopes and the diagnostics config). The revision bump + /// invalidates in-flight background workers (e.g. diagnostics), and + /// triggers a diagnostic refresh. + pub(crate) fn bump_revision(&mut self) { + self.db.synthetic_write(salsa::Durability::LOW); + } + pub(crate) fn open_file_mut(&mut self, uri: &Url) -> anyhow::Result<&mut OpenFile> { let key = FilePath::from_url(uri); if let Some(open_file) = self.open_files.get_mut(&key) { diff --git a/crates/ark/src/lsp/state_handlers.rs b/crates/ark/src/lsp/state_handlers.rs index a249cc2c5d..d7d47714f7 100644 --- a/crates/ark/src/lsp/state_handlers.rs +++ b/crates/ark/src/lsp/state_handlers.rs @@ -506,10 +506,11 @@ async fn update_config( } } - // Refresh diagnostics if the configuration changed + // Refresh diagnostics if the configuration changed. The config lives + // outside Oak so we bump the revision manually. if state.config.diagnostics != diagnostics_config { tracing::info!("Refreshing diagnostics after configuration changed"); - lsp::main_loop::diagnostics_refresh_all(state); + state.bump_revision(); } Ok(()) @@ -526,8 +527,9 @@ pub(crate) fn did_change_console_inputs( // We currently rely on global console scopes for diagnostics, in particular // during package development in conjunction with `devtools::load_all()`. // Ideally diagnostics would not rely on these though, and we wouldn't need - // to refresh from here. - lsp::diagnostics_refresh_all(state); + // to refresh from here. The scopes live outside oak so we bump the revision + // manually. + state.bump_revision(); Ok(()) } diff --git a/crates/ark/src/lsp/tests/state_handlers.rs b/crates/ark/src/lsp/tests/state_handlers.rs index e2e0b6caab..ef8e813119 100644 --- a/crates/ark/src/lsp/tests/state_handlers.rs +++ b/crates/ark/src/lsp/tests/state_handlers.rs @@ -34,8 +34,10 @@ use crate::lsp::main_loop::LspState; use crate::lsp::main_loop::TokioUnboundedSender; use crate::lsp::sources::SourceScheduler; use crate::lsp::state::WorldState; +use crate::lsp::state_handlers::did_change_console_inputs; use crate::lsp::state_handlers::did_close; use crate::lsp::state_handlers::effective_workspace_uris; +use crate::lsp::state_handlers::ConsoleInputs; /// Local sync wrappers around the async-shaped scheduler API. Tests /// don't need the timing flexibility, so each operation kicks off @@ -661,3 +663,26 @@ fn test_did_close_releases_orphan_file_to_stale() { AuxiliaryEvent::PublishDiagnostics(u, diags, _) if u == url && diags.is_empty() )); } + +/// A console-inputs push carries no oak write, but diagnostics read the console +/// scopes it updates. The handler advances the oak revision so the main loop's +/// central refresh (and its snapshot barrier) fires. Pin that: reverting to a +/// direct refresh that doesn't bump the revision would silently stop the +/// central refresh and let a stale-scope pass publish last. +#[test] +fn test_console_inputs_advance_revision() { + let mut state = WorldState::default(); + let before = salsa::plumbing::current_revision(&state.db); + + did_change_console_inputs( + ConsoleInputs { + console_scopes: vec![vec!["foo".to_string()]], + installed_packages: vec![], + }, + &mut state, + ) + .unwrap(); + + let after = salsa::plumbing::current_revision(&state.db); + assert_ne!(before, after); +}