Skip rediscovery for unchanged AST

README.md

@@ -43,41 +43,41 @@ To learn more about using tryke, see the [documentation](https://tryke.dev/).
 ```python
 from typing import Annotated
 
-import tryke as t
+from tryke import Depends, describe, expect, fixture, test
 
 
-@t.fixture(per="scope")
+@fixture(per="scope")
 def database():
     db = {}
     yield db
     db.clear()
 
 
-with t.describe("users"):
+with describe("users"):
 
-    @t.fixture
-    def users(database: Annotated[dict[str, dict[str, str]], t.Depends(database)]):
+    @fixture
+    def users(database: Annotated[dict[str, dict[str, str]], Depends(database)]):
         database["users"] = {}
 
         return database["users"]
 
-    with t.describe("get"):
+    with describe("get"):
 
-        @t.test("returns a stored user")
-        async def test_get(users: Annotated[dict[str, str], t.Depends(users)]):
+        @test("returns a stored user")
+        async def test_get(users: Annotated[dict[str, str], Depends(users)]):
             users["alice"] = "alice@example.com"
 
-            t.expect(users["alice"], name="returns stored email").to_equal(
+            expect(users["alice"], name="returns stored email").to_equal(
                 "alice@example.com"
             )
 
-    with t.describe("set"):
+    with describe("set"):
 
-        @t.test("stores a new user")
-        async def test_set(users: Annotated[dict[str, str], t.Depends(users)]):
+        @test("stores a new user")
+        async def test_set(users: Annotated[dict[str, str], Depends(users)]):
             users["bob"] = "bob@example.com"
 
-            t.expect(users["bob"], name="stores email under user key").to_equal(
+            expect(users["bob"], name="stores email under user key").to_equal(
                 "bob@example.com"
             )
 

crates/tryke_discovery/src/db.rs

@@ -1,5 +1,8 @@
 use std::path::PathBuf;
 
+use log::trace;
+use ruff_python_ast::{ModModule, Stmt};
+use ruff_python_parser::parse_module;
 use tryke_types::ParsedFile;
 
 #[salsa::db]
@@ -17,6 +20,83 @@ pub struct SourceFile {
     pub path: PathBuf,
 }
 
+/// Parsed Python source cached as the first incremental layer.
+///
+/// Equality intentionally ignores raw source text. If the parser produces the
+/// same AST body for a new source string, Salsa keeps the old value and
+/// backdates dependents, so discovery is not re-run for trivia-only edits.
+#[derive(Debug, Clone)]
+pub(crate) struct ParsedAst {
+    source: String,
+    syntax: Option<ModModule>,
+}
+
+impl ParsedAst {
+    pub(crate) fn parse(source: &str) -> Self {
+        let syntax = parse_module(source)
+            .ok()
+            .map(ruff_python_parser::Parsed::into_syntax);
+        Self {
+            source: source.to_owned(),
+            syntax,
+        }
+    }
+
+    pub(crate) fn source(&self) -> &str {
+        &self.source
+    }
+
+    pub(crate) fn syntax(&self) -> Option<&ModModule> {
+        self.syntax.as_ref()
+    }
+
+    fn body(&self) -> Option<&[Stmt]> {
+        self.syntax.as_ref().map(|module| module.body.as_slice())
+    }
+}
+
+impl PartialEq for ParsedAst {
+    fn eq(&self, other: &Self) -> bool {
+        self.body() == other.body()
+    }
+}
+
+#[cfg(test)]
+static DISCOVER_FILE_EXECUTIONS: std::sync::atomic::AtomicUsize =
+    std::sync::atomic::AtomicUsize::new(0);
+
+#[cfg(test)]
+static COUNTED_DISCOVER_FILE_PATH: std::sync::Mutex<Option<PathBuf>> = std::sync::Mutex::new(None);
+
+#[cfg(test)]
+pub(crate) fn count_discover_file_executions_for(path: PathBuf) {
+    *COUNTED_DISCOVER_FILE_PATH.lock().expect("counter mutex") = Some(path);
+    DISCOVER_FILE_EXECUTIONS.store(0, std::sync::atomic::Ordering::SeqCst);
+}
+
+#[cfg(test)]
+pub(crate) fn discover_file_executions() -> usize {
+    DISCOVER_FILE_EXECUTIONS.load(std::sync::atomic::Ordering::SeqCst)
+}
+
+#[cfg(test)]
+fn count_discover_file_execution(path: &std::path::Path) {
+    let counted = COUNTED_DISCOVER_FILE_PATH.lock().expect("counter mutex");
+    if counted.as_deref() == Some(path) {
+        DISCOVER_FILE_EXECUTIONS.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
+    }
+}
+
+#[salsa::tracked(returns(ref))]
+pub(crate) fn parse_file(db: &dyn Db, file: SourceFile) -> ParsedAst {
+    let path = file.path(db);
+    trace!(
+        "parsing {}",
+        path.strip_prefix(file.root(db)).unwrap_or(path).display()
+    );
+    ParsedAst::parse(file.text(db))
+}
+
 /// Everything derivable from a single parse of a Python source file:
 /// the `ParsedFile` (tests, hooks, guard-else lines, errors), the
 /// candidate import paths this file references, and the dynamic-import
@@ -33,11 +113,14 @@ pub struct DiscoveredFile {
 
 #[salsa::tracked]
 pub fn discover_file(db: &dyn Db, file: SourceFile) -> DiscoveredFile {
-    crate::discover_file_from_source(
+    #[cfg(test)]
+    count_discover_file_execution(file.path(db));
+
+    crate::discover_file_from_ast(
         file.root(db),
         file.src_roots(db),
         file.path(db),
-        file.text(db),
+        parse_file(db, file),
     )
 }
 

crates/tryke_discovery/src/discoverer.rs

@@ -756,6 +756,44 @@ mod tests {
         assert_eq!(second.len(), 2);
     }
 
+    #[test]
+    fn discoverer_skips_discovery_when_ast_body_is_unchanged() {
+        let source_one = "from tryke import test\n\n@test\ndef test_one():\n    pass\n# first\n";
+        let dir = make_project(&[("test_example.py", source_one)]);
+        let path = dir
+            .path()
+            .join("test_example.py")
+            .canonicalize()
+            .expect("canonicalize test file");
+        let mut discoverer = Discoverer::new(dir.path());
+        crate::db::count_discover_file_executions_for(path.clone());
+
+        let first = discoverer.rediscover();
+        assert_eq!(first.len(), 1);
+        assert_eq!(crate::db::discover_file_executions(), 1);
+
+        let source_two =
+            "from tryke import test\n\n@test\ndef test_one():\n    pass\n# second comment\n";
+        fs::write(&path, source_two).expect("overwrite with trivia-only change");
+        let second = discoverer.rediscover_changed(std::slice::from_ref(&path));
+        assert_eq!(second.len(), 1);
+        assert_eq!(
+            crate::db::discover_file_executions(),
+            1,
+            "trivia-only source changes should not re-run discovery"
+        );
+
+        let source_three = "from tryke import test\n\n@test\ndef test_one():\n    pass\n\n@test\ndef test_two():\n    pass\n# second comment\n";
+        fs::write(&path, source_three).expect("overwrite with AST change");
+        let third = discoverer.rediscover_changed(std::slice::from_ref(&path));
+        assert_eq!(third.len(), 2);
+        assert_eq!(
+            crate::db::discover_file_executions(),
+            2,
+            "AST changes should re-run discovery"
+        );
+    }
+
     #[test]
     fn discoverer_saves_cache_under_custom_cache_dir() {
         let source = "@test\ndef test_hello():\n    pass\n";

crates/tryke_discovery/src/lib.rs

@@ -17,11 +17,13 @@ pub use discoverer::Discoverer;
 use ignore::WalkBuilder;
 
 use ruff_python_ast::{Expr, Stmt};
-use ruff_python_parser::parse_module;
 use ruff_source_file::LineIndex;
 use ruff_text_size::{Ranged, TextRange, TextSize};
 use tryke_types::{ExpectedAssertion, FixturePer, HookItem, ParsedFile, TestItem};
 
+#[cfg(test)]
+use ruff_python_parser::parse_module;
+
 pub(crate) fn find_project_root(start: &Path) -> Option<PathBuf> {
     start
         .ancestors()
@@ -1837,27 +1839,23 @@ fn collect_doctests_from_body(
     }
 }
 
-/// Parse `source` once and produce everything discovery needs: the
+/// Walk a parsed source file once and produce everything discovery needs: the
 /// `ParsedFile` (tests, hooks, guard-else lines, errors), the project-local
 /// imports this file depends on, and whether it contains dynamic imports.
-/// Folding all three derivations into a single AST walk avoids the prior
-/// cold-start cost of parsing each file twice.
-pub(crate) fn discover_file_from_source(
+/// Keeping this downstream of `db::parse_file` lets Salsa skip discovery when
+/// source text changes but the parsed AST body is unchanged.
+pub(crate) fn discover_file_from_ast(
     root: &Path,
     src_roots: &[PathBuf],
     file: &Path,
-    source: &str,
+    parsed: &crate::db::ParsedAst,
 ) -> crate::db::DiscoveredFile {
-    trace!(
-        "parsing {}",
-        file.strip_prefix(root).unwrap_or(file).display()
-    );
-    let Ok(parsed) = parse_module(source) else {
+    let Some(module) = parsed.syntax() else {
         trace!("parse error in {}", file.display());
         return crate::db::DiscoveredFile::default();
     };
+    let source = parsed.source();
     let line_index = LineIndex::from_source_text(source);
-    let module = parsed.syntax();
     let body = &module.body;
     let aliases = TrykeAliases::collect(body);
     let mut tests = Vec::new();
@@ -1901,7 +1899,8 @@ pub(crate) fn parse_tests_from_source(
     file: &Path,
     source: &str,
 ) -> ParsedFile {
-    discover_file_from_source(root, src_roots, file, source).parsed
+    let parsed = crate::db::ParsedAst::parse(source);
+    discover_file_from_ast(root, src_roots, file, &parsed).parsed
 }
 
 fn parse_tests_from_file(root: &Path, src_roots: &[PathBuf], file: &Path) -> ParsedFile {

docs/index.md

@@ -59,41 +59,41 @@ Or, check out the [tryke playground](https://playground.tryke.dev) to try it out
 ```python
 from typing import Annotated
 
-import tryke as t
+from tryke import Depends, describe, expect, fixture, test
 
 
-@t.fixture(per="scope")
+@fixture(per="scope")
 def database():
     db = {}
     yield db
     db.clear()
 
 
-with t.describe("users"):
+with describe("users"):
 
-    @t.fixture
-    def users(database: Annotated[dict[str, dict[str, str]], t.Depends(database)]):
+    @fixture
+    def users(database: Annotated[dict[str, dict[str, str]], Depends(database)]):
         database["users"] = {}
 
         return database["users"]
 
-    with t.describe("get"):
+    with describe("get"):
 
-        @t.test("returns a stored user")
-        async def test_get(users: Annotated[dict[str, str], t.Depends(users)]):
+        @test("returns a stored user")
+        async def test_get(users: Annotated[dict[str, str], Depends(users)]):
             users["alice"] = "alice@example.com"
 
-            t.expect(users["alice"], name="returns stored email").to_equal(
+            expect(users["alice"], name="returns stored email").to_equal(
                 "alice@example.com"
             )
 
-    with t.describe("set"):
+    with describe("set"):
 
-        @t.test("stores a new user")
-        async def test_set(users: Annotated[dict[str, str], t.Depends(users)]):
+        @test("stores a new user")
+        async def test_set(users: Annotated[dict[str, str], Depends(users)]):
             users["bob"] = "bob@example.com"
 
-            t.expect(users["bob"], name="stores email under user key").to_equal(
+            expect(users["bob"], name="stores email under user key").to_equal(
                 "bob@example.com"
             )