Add doc comments to database layer and AgentApi

Author: oriongonza

src/agent/api.rs

@@ -1,3 +1,5 @@
+//! HTTP client that agents use to communicate with the crater server.
+
 use std::time::Duration;
 
 use crate::agent::Capabilities;
@@ -15,6 +17,7 @@ use reqwest::{Method, StatusCode};
 use serde::de::DeserializeOwned;
 use serde_json::json;
 
+/// Errors returned by the crater server's agent API.
 #[derive(Debug, thiserror::Error)]
 pub enum AgentApiError {
     #[error("invalid API endpoint called")]
@@ -29,6 +32,7 @@ pub enum AgentApiError {
     InternalServerError(String),
 }
 
+/// Converts an HTTP response into a typed `ApiResponse`, mapping status codes to errors.
 trait ResponseExt {
     fn to_api_response<T: DeserializeOwned>(self) -> Fallible<T>;
 }
@@ -72,6 +76,7 @@ pub struct AgentApi {
 }
 
 impl AgentApi {
+    /// Creates a new API client targeting the given server URL with an auth token.
     pub fn new(url: &str, token: &str) -> Self {
         AgentApi {
             url: url.to_string(),
@@ -80,6 +85,7 @@ impl AgentApi {
         }
     }
 
+    /// Builds an authenticated HTTP request to the given agent-api endpoint.
     fn build_request(&self, method: Method, url: &str) -> RequestBuilder {
         utils::http::prepare_sync(method, &format!("{}/agent-api/{url}", self.url)).header(
             AUTHORIZATION,
@@ -90,6 +96,11 @@ impl AgentApi {
         )
     }
 
+    /// Retries a request with exponential backoff on transient failures.
+    // - Retries on ServerUnavailable, reqwest timeouts/connection errors,
+    //   and SQLite "database is locked" errors.
+    // - Backs off from 16 s up to a cap of 8 minutes between attempts.
+    // - Non-transient errors are returned immediately.
     fn retry<T, F: Fn(&Self) -> Fallible<T>>(&self, f: F) -> Fallible<T> {
         let mut retry_interval = 16u64;
         loop {
@@ -126,6 +137,7 @@ impl AgentApi {
         }
     }
 
+    /// Sends the agent's capabilities and receives its configuration from the server.
     pub fn config(&self, caps: &Capabilities) -> Fallible<AgentConfig> {
         self.retry(|this| {
             this.build_request(Method::POST, "config")
@@ -135,6 +147,7 @@ impl AgentApi {
         })
     }
 
+    /// Polls the server for the next experiment to run, sleeping 120 s if none is available.
     pub fn next_experiment(&self) -> Result<Experiment> {
         self.retry(|this| loop {
             let resp: Option<_> = this
@@ -154,6 +167,7 @@ impl AgentApi {
         })
     }
 
+    /// Requests the next crate to test for the given experiment, or `None` if the queue is empty.
     pub fn next_crate(&self, ex: &str) -> Fallible<Option<Crate>> {
         self.retry(|this| {
             let resp: Option<Crate> = this
@@ -166,6 +180,7 @@ impl AgentApi {
         })
     }
 
+    /// Uploads a crate's build/test result and base64-encoded log to the server.
     pub fn record_progress(
         &self,
         ex: &Experiment,
@@ -194,6 +209,7 @@ impl AgentApi {
         })
     }
 
+    /// Sends a heartbeat to the server to signal this agent is still alive.
     pub fn heartbeat(&self) -> Fallible<()> {
         self.retry(|this| {
             let _: bool = this
@@ -207,6 +223,7 @@ impl AgentApi {
         })
     }
 
+    /// Reports an error encountered while running an experiment to the server.
     pub fn report_error(&self, ex: &Experiment, error: String) -> Fallible<()> {
         self.retry(|this| {
             let _: bool = this

src/db/migrations.rs

@@ -1,3 +1,5 @@
+//! Schema migrations applied on database startup.
+
 use crate::prelude::*;
 use rand::distr::{Alphanumeric, SampleString};
 use rusqlite::{Connection, Transaction};
@@ -366,6 +368,11 @@ fn migrations() -> Vec<(&'static str, MigrationKind)> {
     migrations
 }
 
+/// Applies all pending migrations to the database.
+// - Creates the `migrations` tracking table on first run (user_version == 0).
+// - Loads the set of already-executed migration names.
+// - Iterates the full migration list, running each unapplied one inside
+//   its own transaction and recording it in the `migrations` table.
 pub fn execute(db: &mut Connection) -> Fallible<()> {
     // If the database version is 0, create the migrations table and bump it
     let version: i32 = db.query_row("PRAGMA user_version;", [], |r| r.get(0))?;

src/db/mod.rs

@@ -76,6 +76,9 @@ pub struct Database {
 }
 
 impl Database {
+    /// Opens the database at the default path inside [`WORK_DIR`].
+    // - Checks for legacy database filenames and renames if found.
+    // - Delegates to `Database::new` for pool setup and migrations.
     pub fn open() -> Fallible<Self> {
         let path = WORK_DIR.join(DATABASE_PATH);
         if !path.exists() {
@@ -100,6 +103,7 @@ impl Database {
         Database::new(SqliteConnectionManager { file: path }, None)
     }
 
+    /// Opens or creates a database at the given filesystem path.
     pub fn open_at(path: &Path) -> Fallible<Self> {
         std::fs::create_dir_all(&*WORK_DIR)?;
         Database::new(
@@ -121,6 +125,11 @@ impl Database {
         )
     }
 
+    /// Builds the connection pool, configures WAL mode, and runs pending migrations.
+    // - Creates an r2d2 pool with up to 20 connections (covers all production threads).
+    // - Enables WAL journal mode for concurrent reads during writes.
+    // - Sets synchronous=NORMAL for better performance (safe under WAL).
+    // - Runs all pending schema migrations.
     fn new(conn: SqliteConnectionManager, tempfile: Option<NamedTempFile>) -> Fallible<Self> {
         let pool = Pool::builder()
             // By inspection we have 13 threads in production, so make sure each of them can get a
@@ -161,6 +170,7 @@ impl Database {
         })
     }
 
+    /// Runs a closure inside a database transaction, committing on success or rolling back on error.
     pub fn transaction<T, F: FnOnce(&TransactionHandle) -> Fallible<T>>(
         &self,
         will_write: bool,
@@ -194,11 +204,13 @@ pub struct TransactionHandle<'a> {
 }
 
 impl TransactionHandle<'_> {
+    /// Commits the transaction, persisting all changes.
     pub fn commit(self) -> Fallible<()> {
         self.transaction.commit()?;
         Ok(())
     }
 
+    /// Rolls the transaction back, discarding all changes.
     pub fn rollback(self) -> Fallible<()> {
         self.transaction.rollback()?;
         Ok(())
@@ -207,8 +219,10 @@ impl TransactionHandle<'_> {
 
 /// Convenience methods for executing SQL queries against the database.
 pub trait QueryUtils {
+    /// Acquires a connection and passes it to the closure.
     fn with_conn<T, F: FnOnce(&Connection) -> Fallible<T>>(&self, f: F) -> Fallible<T>;
 
+    /// Returns `true` if the query matches at least one row.
     fn exists(&self, sql: &str, params: &[&dyn ToSql]) -> Fallible<bool> {
         self.with_conn(|conn| {
             self.trace(sql, || {
@@ -218,6 +232,7 @@ pub trait QueryUtils {
         })
     }
 
+    /// Executes a statement and returns the number of rows changed.
     fn execute(&self, sql: &str, params: &[&dyn ToSql]) -> Fallible<usize> {
         self.with_conn(|conn| {
             self.trace(sql, || {
@@ -228,6 +243,7 @@ pub trait QueryUtils {
         })
     }
 
+    /// Like [`execute`](Self::execute), but uses a prepared-statement cache.
     fn execute_cached(&self, sql: &str, params: &[&dyn ToSql]) -> Fallible<usize> {
         self.with_conn(|conn| {
             self.trace(sql, || {
@@ -238,6 +254,7 @@ pub trait QueryUtils {
         })
     }
 
+    /// Returns the first row of a query, or `None` if the result set is empty.
     fn get_row<T, P>(
         &self,
         sql: &str,
@@ -261,6 +278,7 @@ pub trait QueryUtils {
         })
     }
 
+    /// Executes a query and collects all rows into a `Vec`.
     fn query<T, F: FnMut(&Row) -> rusqlite::Result<T>>(
         &self,
         sql: &str,
@@ -282,6 +300,7 @@ pub trait QueryUtils {
         })
     }
 
+    /// Returns the first row mapped through a fallible closure, or `None`.
     fn query_row<T, F: FnOnce(&Row) -> Fallible<T>>(
         &self,
         sql: &str,
@@ -300,6 +319,7 @@ pub trait QueryUtils {
         })
     }
 
+    /// Runs a closure and logs the SQL statement if it takes longer than 500 ms.
     fn trace<T, F: FnOnce() -> T>(&self, sql: &str, f: F) -> T {
         let start = Instant::now();
         let res = f();