POC: package_by_file_query() rather than package field in File

crates/oak_db/src/db.rs

@@ -178,6 +178,29 @@ pub fn root_by_package_query(db: &dyn Db, pkg: Package) -> Option<Root> {
     best.map(|(root, _)| root)
 }
 
+/// Resolve the [`Package`] that owns `file`, if any.
+///
+/// Walks live roots' `File -> Package` indexes in workspace-then-library
+/// order and returns the first hit. A file belongs to at most one package
+/// *entity* (in the nested-root case both roots' `packages` hold the same
+/// entity), so first-hit is unambiguous. The orphan bucket has no packages
+/// and contributes nothing; stale entities are invisible by design, which
+/// is what makes an evicted file's package association clear to `None`.
+///
+/// Backs [`crate::File::package`], which is the derived replacement for the
+/// old `File.package` back-pointer field: the container vecs are now the
+/// single source of truth for file ownership.
+pub fn package_by_file_query(db: &dyn Db, file: File) -> Option<Package> {
+    for &root in db.live_roots() {
+        if let LiveRoot::Workspace(r) | LiveRoot::Library(r) = root {
+            if let Some(&pkg) = root_file_package_index(db, r).get(&file) {
+                return Some(pkg);
+            }
+        }
+    }
+    None
+}
+
 /// Number of path segments in a root's URL. Used as the tiebreaker by
 /// [`root_by_package_query`] when nested roots both claim the same package.
 ///
@@ -212,6 +235,21 @@ fn root_url_index(db: &dyn Db, root: Root) -> FxHashMap<UrlId, File> {
     map
 }
 
+/// Per-root File -> owning Package index. Built from `root.packages` and
+/// each package's `files`. Same per-root granularity as [`root_url_index`]:
+/// adding or removing a file in this root invalidates only this entry.
+/// Backs [`package_by_file_query`].
+#[salsa::tracked(returns(ref))]
+fn root_file_package_index(db: &dyn Db, root: Root) -> FxHashMap<File, Package> {
+    let mut map = FxHashMap::default();
+    for &pkg in root.packages(db) {
+        for &file in pkg.files(db) {
+            map.insert(file, pkg);
+        }
+    }
+    map
+}
+
 /// Orphan URL -> File index. Reads only `orphan_root().files`.
 #[salsa::tracked(returns(ref))]
 fn orphan_url_index(db: &dyn Db) -> FxHashMap<UrlId, File> {

crates/oak_db/src/file.rs

@@ -23,36 +23,18 @@ use crate::Root;
 /// The `url` field is a [`UrlId`], so the type system enforces "everything
 /// inside Salsa is a canonical URL".
 ///
-/// `package` is a back-pointer to the [`Package`] this file belongs to, or
-/// `None` for standalone scripts. Inverse of `Package.files`, so queries
-/// answering "what package owns this file?" don't walk the forward edge.
-/// Files with `package == None` are either standalone scripts under a
-/// workspace root or orphan files not registered anywhere.
-///
-/// # Placement invariant
-///
-/// `File.package` and the file's physical location in a `Vec<File>` are
-/// expected to agree. A file with `package == Some(pkg)` should live in
-/// `pkg.files`. A file with `package == None` should live in either some
-/// `root.scripts` or `orphan_root().files`. The salsa setters (`set_url`,
-/// `set_contents`, `set_package`) are `pub` because field visibility couples to
-/// setter visibility in salsa but calling `set_package` directly leaves the
-/// file in its old bucket and silently breaks this invariant.
-///
-/// The scanner crate (`oak_scan`) wraps these setters in helpers that
-/// maintain placement (move the file between `pkg.files`,
-/// `root.scripts`, and `orphan_root().files` as `package` changes).
-/// Callers that go around the helpers and use the salsa setters
-/// directly must maintain placement themselves.
+/// File ownership has a single source of truth: the forward edge. A file
+/// belongs to whichever container currently holds it -- `pkg.files`,
+/// `root.scripts`, or `orphan_root().files`. There is no stored
+/// back-pointer to keep in sync. "What package owns this file?" is
+/// answered by the derived [`File::package`] query, which walks the
+/// per-root `File -> Package` indexes; "what root?" by [`File::root`].
 #[salsa::input(debug)]
 pub struct File {
     #[returns(ref)]
     pub url: UrlId,
     #[returns(ref)]
     pub contents: String,
-    /// **Placement invariant.** Call this setter only through
-    /// `oak_scan`'s helpers; see the type-level doc above.
-    pub package: Option<Package>,
 }
 
 #[salsa::tracked]
@@ -144,9 +126,23 @@ impl File {
             .collect()
     }
 
+    /// The [`Package`] that owns this file, or `None` for standalone
+    /// scripts and orphan / stale files.
+    ///
+    /// Derived from live-graph containment via
+    /// [`package_by_file_query`](crate::db::package_by_file_query): the
+    /// file belongs to whichever live `pkg.files` currently holds it.
+    /// This replaces the old `File.package` back-pointer field, so file
+    /// ownership has a single source of truth (the forward edge) and no
+    /// invariant to maintain by hand.
+    #[salsa::tracked]
+    pub fn package(self, db: &dyn Db) -> Option<Package> {
+        crate::db::package_by_file_query(db, self)
+    }
+
     /// The root containing this file, if any.
     ///
-    /// If the file has a registered [`Package`], asks the db which live
+    /// If the file has an owning [`Package`], asks the db which live
     /// root holds it via [`Db::root_by_package`]. Otherwise falls back to a
     /// URL-prefix lookup against [`WorkspaceRoots`] (orphan files live
     /// under a workspace root or nowhere). Library files normally have

crates/oak_db/src/file_imports.rs

@@ -128,10 +128,11 @@ fn narrow_script_top_level(file: File, db: &dyn Db, offset: TextSize) -> Vec<Imp
 fn narrow_package_top_level(file: File, db: &dyn Db, package: Package) -> Vec<ImportLayer> {
     let files = package.files(db);
     let Some(file_pos) = files.iter().position(|f| *f == file) else {
-        // File claims membership but isn't in the package's `files`.
+        // `package` was derived from `package.files` containing this file
+        // (via `File::package`), so the position is always found.
         // Shouldn't happen.
         log::warn!(
-            "File {file} has package back-pointer to {package} but is not in its files",
+            "File {file} resolved to package {package} but is not in its files",
             file = file.url(db),
             package = package.name(db),
         );

crates/oak_db/src/inputs.rs

@@ -23,14 +23,14 @@ pub struct Root {
     pub path: UrlId,
     pub kind: RootKind,
     /// Top-level R scripts directly under this root. Each entry is a
-    /// `File` with `package(db) == None`. Always empty for `Library`
-    /// roots.
+    /// `File` not owned by any package (`package(db) == None`). Always
+    /// empty for `Library` roots.
     ///
-    /// **Placement invariant.** A file present here must have
-    /// `package(db) == None`, and a file with `package == None` must
-    /// live here, in another `Root.scripts`, or in
-    /// `OrphanRoot.files`. Call this setter only through `oak_scan`'s
-    /// helpers, which keep the back-pointer and the container in sync.
+    /// A file should live in exactly one container (`Root.scripts`,
+    /// `Package.files`, or `OrphanRoot.files`); `File::package` derives
+    /// ownership from that. Push files here only through `oak_scan`'s
+    /// helpers, which move a file out of its old container as it changes
+    /// homes.
     #[returns(ref)]
     pub scripts: Vec<File>,
     /// Packages discovered under this root (workspace packages for
@@ -121,9 +121,10 @@ impl LibraryRoots {
 /// [`crate::Db::file_by_url`] consults to find unanchored files.
 #[salsa::input(debug)]
 pub struct OrphanRoot {
-    /// **Placement invariant.** Files here must have `package(db) ==
-    /// None`. Call this setter only through `oak_scan`'s helpers,
-    /// which keep the back-pointer and the container in sync.
+    /// Files here are owned by no package: since the orphan bucket has no
+    /// `packages`, `File::package` derives `None` for them. Push files
+    /// here only through `oak_scan`'s helpers, which move a file out of
+    /// its old container as it changes homes.
     #[returns(ref)]
     pub files: Vec<File>,
 }
@@ -204,10 +205,11 @@ pub struct Package {
     /// package doesn't invalidate tracked queries reading another
     /// package's files.
     ///
-    /// **Placement invariant.** A file present here must have
-    /// `package(db) == Some(self)`. Call this setter only through
-    /// `oak_scan`'s helpers, which keep the back-pointer and the
-    /// container in sync.
+    /// This vec is the source of truth for package membership:
+    /// `File::package` derives ownership by finding the file here. A file
+    /// should live in exactly one container, so push files here only
+    /// through `oak_scan`'s helpers, which move a file out of its old
+    /// container as it changes homes.
     #[returns(ref)]
     pub files: Vec<File>,
     /// The basename ordering from `DESCRIPTION`'s `Collate` field, if

crates/oak_db/src/tests/db.rs

@@ -14,7 +14,7 @@ use crate::Package;
 fn test_file_by_url_finds_workspace_script() {
     let mut db = TestDb::new();
     let root = workspace_root(&db, "proj");
-    let file = File::new(&db, file_url("proj/a.R"), String::new(), None);
+    let file = File::new(&db, file_url("proj/a.R"), String::new());
     root.set_scripts(&mut db).to(vec![file]);
     db.workspace_roots().set_roots(&mut db).to(vec![root]);
 
@@ -25,10 +25,9 @@ fn test_file_by_url_finds_workspace_script() {
 fn test_file_by_url_finds_workspace_package_file() {
     let mut db = TestDb::new();
     let root = workspace_root(&db, "proj");
-    // Construct the back-pointer + forward-edge pair: create the
-    // `Package` with an empty `files` list, then the `File` with the
-    // `package` set, then attach the file via `set_files`. Matches the
-    // shape `oak_scan`'s placement-preserving helpers will produce.
+    // Attach the file to the package via the forward edge (`set_files`),
+    // the single source of truth for membership. Matches the shape
+    // `oak_scan`'s placement-preserving helpers produce.
     let pkg = Package::new(
         &db,
         file_url("proj/DESCRIPTION"),
@@ -38,7 +37,7 @@ fn test_file_by_url_finds_workspace_package_file() {
         vec![],
         None,
     );
-    let file = File::new(&db, file_url("proj/R/foo.R"), String::new(), Some(pkg));
+    let file = File::new(&db, file_url("proj/R/foo.R"), String::new());
     pkg.set_files(&mut db).to(vec![file]);
     root.set_packages(&mut db).to(vec![pkg]);
     db.workspace_roots().set_roots(&mut db).to(vec![root]);
@@ -59,7 +58,7 @@ fn test_file_by_url_finds_library_package_file() {
         vec![],
         None,
     );
-    let file = File::new(&db, file_url("libs/foo/R/a.R"), String::new(), Some(pkg));
+    let file = File::new(&db, file_url("libs/foo/R/a.R"), String::new());
     pkg.set_files(&mut db).to(vec![file]);
     lib.set_packages(&mut db).to(vec![pkg]);
     db.library_roots().set_roots(&mut db).to(vec![lib]);
@@ -70,7 +69,7 @@ fn test_file_by_url_finds_library_package_file() {
 #[test]
 fn test_file_by_url_finds_orphan_file() {
     let mut db = TestDb::new();
-    let file = File::new(&db, file_url("untitled.R"), String::new(), None);
+    let file = File::new(&db, file_url("untitled.R"), String::new());
     db.orphan_root().set_files(&mut db).to(vec![file]);
 
     assert_eq!(db.file_by_url(&file_url("untitled.R")), Some(file));
@@ -90,8 +89,8 @@ fn test_root_url_index_invalidates_per_root() {
     let mut db = TestDb::new();
     let root_a = workspace_root(&db, "a");
     let root_b = workspace_root(&db, "b");
-    let file_a = File::new(&db, file_url("a/file.R"), String::new(), None);
-    let file_b = File::new(&db, file_url("b/file.R"), String::new(), None);
+    let file_a = File::new(&db, file_url("a/file.R"), String::new());
+    let file_b = File::new(&db, file_url("b/file.R"), String::new());
     root_a.set_scripts(&mut db).to(vec![file_a]);
     root_b.set_scripts(&mut db).to(vec![file_b]);
     db.workspace_roots()
@@ -106,7 +105,7 @@ fn test_root_url_index_invalidates_per_root() {
     assert_eq!(db.executions("root_url_index"), 2);
 
     // Add a script to root B. B's index invalidates; A's stays cached.
-    let file_b2 = File::new(&db, file_url("b/other.R"), String::new(), None);
+    let file_b2 = File::new(&db, file_url("b/other.R"), String::new());
     root_b.set_scripts(&mut db).to(vec![file_b, file_b2]);
 
     // Look up the file in A. A's index is still cached, no re-exec.

crates/oak_db/src/tests/file.rs

@@ -9,7 +9,7 @@ use crate::File;
 /// touching the orphan/workspace bucketing logic that's exercised in
 /// `oak_storage/tests/`.
 fn new_file(db: &mut TestDb, name: &str, contents: &str) -> File {
-    File::new(db, file_url(name), contents.to_string(), None)
+    File::new(db, file_url(name), contents.to_string())
 }
 
 #[test]
@@ -56,7 +56,7 @@ fn test_semantic_index_matches_oak_semantic() {
     let db = TestDb::new();
     let source = "x <- 1\nx\n";
     let url = file_url("a.R");
-    let file = File::new(&db, url.clone(), source.to_string(), None);
+    let file = File::new(&db, url.clone(), source.to_string());
 
     let via_salsa = file.semantic_index(&db);
 

crates/oak_db/src/tests/file_exports.rs

@@ -16,7 +16,7 @@ fn setup_workspace(db: &mut TestDb, scripts: &[(&str, &str)]) -> Vec<File> {
     let root = workspace_root(db, "w");
     let files: Vec<File> = scripts
         .iter()
-        .map(|(name, contents)| File::new(db, file_url(name), contents.to_string(), None))
+        .map(|(name, contents)| File::new(db, file_url(name), contents.to_string()))
         .collect();
     root.set_scripts(db).to(files.clone());
     db.workspace_roots().set_roots(db).to(vec![root]);

crates/oak_db/src/tests/file_imports.rs

@@ -51,7 +51,7 @@ fn test_script_with_no_attaches_returns_only_default_search_path() {
     let base = packages[0];
     let stats = packages[1];
 
-    let file = File::new(&db, file_url("a.R"), "x <- 1\n".to_string(), None);
+    let file = File::new(&db, file_url("a.R"), "x <- 1\n".to_string());
     let layers = file.imports(&db);
 
     // Only `stats` and `base` are registered in this test; the other
@@ -77,7 +77,6 @@ fn test_script_attach_produces_package_exports_layer_in_lifo_order() {
         &db,
         file_url("a.R"),
         "library(dplyr)\nlibrary(ggplot2)\n".to_string(),
-        None,
     );
     let layers = file.imports(&db);
 
@@ -98,7 +97,7 @@ fn test_script_attach_produces_package_exports_layer_in_lifo_order() {
 fn test_script_attach_to_unregistered_package_drops_layer() {
     let db = TestDb::new();
     // No `dplyr` in any library root.
-    let file = File::new(&db, file_url("a.R"), "library(dplyr)\n".to_string(), None);
+    let file = File::new(&db, file_url("a.R"), "library(dplyr)\n".to_string());
 
     let layers = file.imports(&db);
     assert!(layers.is_empty());
@@ -132,18 +131,8 @@ fn test_package_file_emits_namespace_and_collation_layers() {
         Vec::new(),
         None,
     );
-    let first = File::new(
-        &db,
-        file_url("w/pkg/R/_a.R"),
-        "first <- 1\n".to_string(),
-        Some(pkg),
-    );
-    let second = File::new(
-        &db,
-        file_url("w/pkg/R/b.R"),
-        "second <- 2\n".to_string(),
-        Some(pkg),
-    );
+    let first = File::new(&db, file_url("w/pkg/R/_a.R"), "first <- 1\n".to_string());
+    let second = File::new(&db, file_url("w/pkg/R/b.R"), "second <- 2\n".to_string());
     pkg.set_files(&mut db).to(vec![first, second]);
     workspace.set_packages(&mut db).to(vec![pkg]);
     db.workspace_roots().set_roots(&mut db).to(vec![workspace]);
@@ -189,7 +178,7 @@ fn test_imports_is_cached_per_file() {
     let mut db = TestDb::new();
     let _ = install_packages(&mut db, &["dplyr"]);
 
-    let file = File::new(&db, file_url("a.R"), "library(dplyr)\n".to_string(), None);
+    let file = File::new(&db, file_url("a.R"), "library(dplyr)\n".to_string());
     let _ = file.imports(&db);
     let _ = file.imports(&db);