docs: describe current behavior, not change history, in comments

src/config.rs

@@ -484,11 +484,10 @@ mod tests {
 
     #[test]
     fn legacy_api_key_in_yaml_is_ignored_on_load() {
-        // Older configs (pre-jwt-branch) had `api_key: hd_xxx` written
-        // to disk. After the migration, the api_key field is purely
-        // transient — `#[serde(skip)]` must drop any value present in
-        // YAML on load. This pins down the migration behavior so a
-        // stale entry can't silently reappear in profile.api_key.
+        // Some config files on disk carry `api_key: hd_xxx`. The api_key field
+        // is transient (`#[serde(skip)]`), so any value present in the YAML must
+        // be dropped on load — a stale entry must not silently reappear in
+        // profile.api_key.
         let (_tmp, _guard) = with_temp_config_dir();
         let path = config_path().unwrap();
         fs::create_dir_all(path.parent().unwrap()).unwrap();

src/connections.rs

@@ -34,8 +34,8 @@ impl Serialize for HealthStatus {
     }
 }
 
-/// Render an [`ApiError`] the way the old raw `fetch_health` path did: the
-/// server body through `api_error`, or the transport message verbatim.
+/// Render an [`ApiError`] as text: the server body through `api_error`, or the
+/// transport message verbatim.
 fn error_text(e: ApiError) -> String {
     match e {
         ApiError::Status { body, .. } => crate::util::api_error(body),
@@ -138,8 +138,7 @@ pub fn types_get(workspace_id: &str, name: &str, format: &str) {
     let api = Api::new(Some(workspace_id));
     let resp = block(api.client().connection_types().get(name)).unwrap_or_else(|e| e.exit());
     // The SDK models nullable fields as `Option<Option<Value>>`; flatten and
-    // drop an explicit JSON `null` to match the old behavior (the old struct
-    // deserialized a missing/`null` field to `None`).
+    // drop an explicit JSON `null` so a missing or null field renders as absent.
     let detail = ConnectionTypeDetail {
         name: resp.name,
         label: resp.label,

src/connections_new.rs

@@ -30,7 +30,7 @@ fn fetch_types(api: &Api) -> Vec<ConnectionTypeSummary> {
 fn fetch_detail(api: &Api, name: &str) -> ConnectionTypeDetail {
     let detail = block(api.client().connection_types().get(name)).unwrap_or_else(|e| e.exit());
     // The SDK models nullable fields as `Option<Option<Value>>`; flatten and
-    // treat an explicit JSON `null` as absent to match the old `is_null()` check.
+    // treat an explicit JSON `null` as absent.
     let flatten = |field: Option<Option<Value>>| -> Option<Value> {
         field.flatten().filter(|v| !v.is_null())
     };
@@ -317,8 +317,8 @@ pub fn run(workspace_id: &str) {
         Unavailable(String),
     }
 
-    /// Render an [`ApiError`] the way the old raw paths did: the server body
-    /// through `api_error`, or the transport message verbatim.
+    /// Render an [`ApiError`] as text: the server body through `api_error`, or
+    /// the transport message verbatim.
     fn error_text(e: ApiError) -> String {
         match e {
             ApiError::Status { body, .. } => crate::util::api_error(body),

src/context.rs

@@ -119,7 +119,7 @@ fn local_md_path(name: &str) -> PathBuf {
 }
 
 /// Fetch a named context document. Returns `Ok(None)` on 404 (not found);
-/// exits the process on any other error, matching the old behavior.
+/// exits the process on any other error.
 fn fetch_context(api: &Api, database_id: &str, name: &str) -> Option<DatabaseContextEntry> {
     let result = crate::sdk::block(api.client().database_context().get(database_id, name));
     match crate::sdk::none_if_404(result) {

src/databases.rs

@@ -131,8 +131,8 @@ impl From<hotdata::models::DatabaseSummary> for DatabaseSummary {
 /// Fetch a database by id through the SDK's typed `databases().get` handle.
 ///
 /// The handle owns auth, scope headers, URL construction, and percent-encoding
-/// of the id segment, so callers no longer hand-roll the path. The result is
-/// mapped into the CLI's `Database`.
+/// of the id segment, so the caller passes a bare id. The result is mapped into
+/// the CLI's `Database`.
 pub(crate) fn get_database(api: &Api, id: &str) -> Result<Database, ApiError> {
     block(api.client().databases().get(id)).map(Database::from)
 }
@@ -579,9 +579,8 @@ fn mint_database_token(api: &Api, database_id: &str) -> DatabaseTokenResponse {
         .post_raw("/auth/database", &body)
         .unwrap_or_else(|e| e.exit());
     if !status.is_success() {
-        // The old typed `api.post` routed non-success through `fail_response`,
-        // which upgrades a masked 401/403/404 into the re-auth hint. Reproduce
-        // that via the seam's auth-aware exit.
+        // Route non-success through the seam's auth-aware exit, which upgrades a
+        // masked 401/403/404 into the re-auth hint.
         crate::sdk::ApiError::Status {
             status,
             body: resp_body,

src/jwt.rs

@@ -368,11 +368,11 @@ pub fn ensure_access_token(
 
 /// Which credential source the [`CliTokenProvider`] serves bearers from.
 ///
-/// Mirrors the 4-level auth-source precedence the old `ApiClient::new`
-/// applied (database env -> sandbox env -> on-disk sandbox session ->
-/// user session/api_key). The wrapper (`src/sdk.rs`) picks the variant at
-/// construction time; the provider re-runs the corresponding *existing*
-/// blocking CLI function on every request so session.json, the 30s leeway
+/// Carries the 4-level auth-source precedence (database env -> sandbox env ->
+/// on-disk sandbox session -> user session/api_key). The wrapper (`src/sdk.rs`)
+/// picks the variant at construction time; the provider re-runs the
+/// corresponding blocking CLI function on every request so session.json, the
+/// 30s leeway
 /// table, no-clobber for Flag/Env, and clear-on-dead-refresh stay owned by
 /// the CLI — the SDK never re-implements JWT exchange.
 #[derive(Debug, Clone)]
@@ -446,8 +446,7 @@ impl hotdata::auth::BearerTokenProvider for CliTokenProvider {
         resolved.map_err(|body| {
             // Surface as a 401 so `Configuration::resolve_bearer_token` logs the
             // cause and the request proceeds to a 401 the wrapper shapes into
-            // the "run hotdata auth" hint (the same end-state as the old
-            // ApiClient refresher returning None).
+            // the "run hotdata auth" hint.
             hotdata::auth::TokenExchangeError::Status { status: 401, body }
         })
     }

src/sandbox.rs

@@ -25,11 +25,10 @@ fn now_unix() -> u64 {
 
 /// Mint (or re-mint) a sandbox-scoped JWT via `POST /v1/auth/sandbox`.
 ///
-/// This token-mint endpoint has no SDK operation, so it stays on the raw seam.
-/// [`Api::post_raw`] still carries the user bearer + `X-Workspace-Id` like every
-/// SDK call. Reproduces the old `ApiClient::post` behavior exactly: a transport
-/// error or a non-success status prints the standard error and exits, and a
-/// malformed body exits the same way `parse_json` did.
+/// This token-mint endpoint has no SDK operation, so it uses the raw seam.
+/// [`Api::post_raw`] carries the user bearer + `X-Workspace-Id` like every SDK
+/// call. A transport error or a non-success status prints the standard error
+/// and exits; a malformed body exits the same way.
 fn mint_sandbox_token(api: &Api, body: &serde_json::Value) -> SandboxTokenResponse {
     let (status, resp_body) = api
         .post_raw("/auth/sandbox", body)
@@ -191,8 +190,8 @@ pub fn new(workspace_id: &str, name: Option<&str>, format: &str) {
 
     // POST /auth/sandbox creates the sandbox AND mints a sandbox-scoped
     // JWT (+ refresh token) in one round-trip. This token-mint endpoint has
-    // no SDK operation, so it stays on the raw seam (which still carries the
-    // user bearer + X-Workspace-Id like every SDK call).
+    // no SDK operation, so it uses the raw seam (which carries the user bearer
+    // + X-Workspace-Id like every SDK call).
     let resp = mint_sandbox_token(&api, &body);
     let sandbox_id = resp.sandbox_id.clone();
     persist_sandbox_session(resp, workspace_id);

src/sdk.rs

@@ -1,9 +1,8 @@
 //! Synchronous wrapper over the async Hotdata Rust SDK.
 //!
-//! This module is the seam that replaces the hand-rolled legacy
-//! `ApiClient`. The 15 command modules stay
-//! synchronous and call [`Api`] methods; [`Api`] drives the async SDK behind a
-//! process-global multi-thread tokio runtime via `block_on`.
+//! This module is the seam between the CLI and the SDK. The 15 command modules
+//! stay synchronous and call [`Api`] methods; [`Api`] drives the async SDK
+//! behind a process-global multi-thread tokio runtime via `block_on`.
 //!
 //! # Concurrency contract
 //!
@@ -17,8 +16,8 @@
 //!
 //! # Auth
 //!
-//! Construction reproduces the old `ApiClient::new` 4-level auth-source
-//! precedence by choosing the [`AuthMode`](crate::jwt::AuthMode) the installed
+//! Construction follows a 4-level auth-source precedence, choosing the
+//! [`AuthMode`](crate::jwt::AuthMode) the installed
 //! [`CliTokenProvider`](crate::jwt::CliTokenProvider) will serve. The provider
 //! returns a ready CLI-minted JWT (`client_id=hotdata-cli`, `/o/token/`), which
 //! the SDK passes through unchanged; the CLI keeps full ownership of
@@ -69,11 +68,12 @@ pub struct Api {
     database_id: Option<String>,
 }
 
-/// Request timeout for SDK-routed calls. Mirrors the old `ApiClient` so a hung
-/// server cannot stall the CLI indefinitely. The streaming `/files` upload
-/// keeps its own no-timeout client on the raw-HTTP path.
+/// Request timeout for SDK-routed calls, so a hung server cannot stall the CLI
+/// indefinitely. The streaming `/files` upload runs on its own no-timeout
+/// client ([`upload_reqwest_client`]), since a large upload legitimately
+/// outlives this cap.
 const HTTP_REQUEST_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(300);
-/// TCP keepalive probe interval, matching the old client.
+/// TCP keepalive probe interval.
 const TCP_KEEPALIVE_INTERVAL: std::time::Duration = std::time::Duration::from_secs(30);
 
 /// Build the `reqwest::Client` backing every SDK call, with a request timeout +
@@ -152,8 +152,8 @@ const _: fn() = || {
 
 /// SDK -> CLI error after mapping an `Error<T>`.
 ///
-/// Carries enough to reproduce the old `fail_response` behavior: the HTTP
-/// status and a printable body, or a transport/parse description.
+/// Carries enough for the CLI's failure rendering: the HTTP status and a
+/// printable body, or a transport/parse description.
 #[derive(Debug)]
 pub enum ApiError {
     /// The server returned a non-success status.
@@ -170,8 +170,8 @@ impl ApiError {
     ///
     /// `ResponseError` carries the HTTP status + raw body the CLI's
     /// `format_fail_message` consumes; everything else collapses to a
-    /// transport description (the old "error connecting to API" / "error
-    /// parsing response" paths).
+    /// transport description (the "error connecting to API" / "error parsing
+    /// response" messages).
     pub fn from_sdk<T: std::fmt::Debug>(err: Error<T>) -> Self {
         match err {
             Error::ResponseError(ResponseContent {
@@ -253,8 +253,8 @@ where
 
 /// Map a result, returning `Ok(None)` on HTTP 404 instead of an error.
 ///
-/// Reproduces `ApiClient::get_none_if_not_found` / the context-404 / indexes-404
-/// semantics: a missing resource is normal for these probes.
+/// Used by the context-404 / indexes-404 probes, where a missing resource is
+/// normal rather than an error.
 pub fn none_if_404<T>(r: Result<T, ApiError>) -> Result<Option<T>, ApiError> {
     match r {
         Ok(v) => Ok(Some(v)),
@@ -317,10 +317,9 @@ async fn apply_seam_headers(
 }
 
 impl Api {
-    /// Build an [`Api`], reproducing `ApiClient::new`'s auth-source precedence
-    /// by selecting the [`AuthMode`] the installed provider will serve. Exits
-    /// with a diagnostic if config can't load or no usable credential exists,
-    /// matching the old startup behavior.
+    /// Build an [`Api`], selecting the [`AuthMode`] the installed provider will
+    /// serve from the auth-source precedence below. Exits with a diagnostic if
+    /// config can't load or no usable credential exists.
     pub fn new(workspace_id: Option<&str>) -> Self {
         let profile_config = match config::load("default") {
             Ok(c) => c,
@@ -331,15 +330,15 @@ impl Api {
         };
         let api_url = profile_config.api_url.to_string();
 
-        // Auth-source precedence (verbatim from the old ApiClient::new):
+        // Auth-source precedence:
         //   1. HOTDATA_DATABASE_TOKEN env (databases run child)
         //   2. HOTDATA_SANDBOX_TOKEN env (sandbox run child)
         //   3. ~/.hotdata/sandbox_session.json present (sandbox set <id>)
         //   4. ~/.hotdata/session.json + optional api_key fallback
         //
-        // We pre-flight the same way the old client did (so a dead/unusable
-        // credential exits at startup with the right hint), then hand the
-        // CliTokenProvider the matching mode to re-resolve on every request.
+        // Pre-flight the chosen source here (so a dead/unusable credential
+        // exits at startup with the right hint), then hand the CliTokenProvider
+        // the matching mode to re-resolve on every request.
         let mode = if std::env::var("HOTDATA_DATABASE_TOKEN").is_ok() {
             if crate::database_session::refresh_from_env(&api_url).is_none() {
                 eprintln!(
@@ -400,8 +399,8 @@ impl Api {
             }
         };
 
-        // Resolve the sandbox/session id exactly as the old ApiClient::new did:
-        // HOTDATA_SANDBOX wins; otherwise, if we are a descendant of a
+        // Resolve the sandbox/session id: HOTDATA_SANDBOX wins; otherwise, if
+        // we are a descendant of a
         // `sandbox run` whose sandbox context was lost, exit (a restart is
         // required); else fall back to the persisted sandbox in config. This id
         // is sent as X-Session-Id to scope requests to the sandbox.
@@ -439,8 +438,7 @@ impl Api {
             base_path: sdk_base_path(api_url),
             client: sdk_http_client(),
             // Attribute CLI traffic as the CLI, not the SDK default
-            // (`hotdata-rust/...`). The old ApiClient sent no User-Agent; an
-            // explicit CLI agent is the correct attribution.
+            // (`hotdata-rust/...`), so server-side request logs name the CLI.
             user_agent: Some(format!("hotdata-cli/{}", env!("CARGO_PKG_VERSION"))),
             ..Configuration::default()
         };
@@ -681,10 +679,9 @@ impl Api {
     /// `Configuration`, returning the raw status + body text.
     ///
     /// The seam's DELETE counterpart to [`post_raw`](Self::post_raw): used by
-    /// `databases.rs`, where the delete bodies feed the same CLI-side
-    /// `(status, body)` control flow as the old raw `delete_raw` (e.g. the
-    /// delete+recreate path inspects the failure body), so non-success is
-    /// returned as `Ok((status, body))` rather than an error.
+    /// `databases.rs`, where the delete+recreate path inspects the failure
+    /// body, so non-success is returned as `Ok((status, body))` rather than an
+    /// error for the caller's `(status, body)` control flow.
     pub fn delete_raw(&self, path: &str) -> Result<(reqwest::StatusCode, String), ApiError> {
         let cfg = self.client.configuration();
         let url = format!("{}/v1{path}", cfg.base_path);
@@ -733,8 +730,6 @@ impl Api {
 
 /// Decide what error text to print for a failed response. Pure function so the
 /// 4xx-to-re-auth-hint heuristic is unit-testable without HTTP or `exit`.
-///
-/// Relocated verbatim from the old `api.rs`.
 pub fn format_fail_message(
     status: reqwest::StatusCode,
     body: &str,
@@ -753,7 +748,7 @@ mod tests {
     use super::*;
     use auth::AuthStatus;
 
-    // --- format_fail_message: ported verbatim from api.rs (9 cases) ----------
+    // --- format_fail_message (9 cases) ---------------------------------------
 
     #[test]
     fn format_fail_message_401_with_invalid_key_shows_reauth_hint() {
@@ -919,8 +914,8 @@ mod tests {
 
     #[test]
     fn workspace_id_header_is_installed_on_scoped_calls() {
-        // Regression for the old api.rs:598 header assertion. `datasets().list`
-        // carries the X-Workspace-Id api_key; assert it reaches the wire.
+        // `datasets().list` carries the X-Workspace-Id api_key; assert it
+        // reaches the wire.
         let mut server = mockito::Server::new();
         let m = server
             .mock("GET", "/v1/datasets")
@@ -1076,8 +1071,8 @@ mod tests {
 
     #[test]
     fn post_raw_returns_error_status_without_mapping_to_err() {
-        // Non-success is returned as Ok((status, body)) so the caller reproduces
-        // the old `(status, body)` raw-post control flow verbatim.
+        // Non-success is returned as Ok((status, body)) so the caller can drive
+        // its own `(status, body)` control flow (e.g. inspect the failure body).
         let mut server = mockito::Server::new();
         let _m = server
             .mock("POST", "/v1/refresh")
@@ -1247,9 +1242,8 @@ mod tests {
 
     #[test]
     fn database_scope_sends_x_database_id_on_raw_calls() {
-        // Regression: `hotdata query --database X` must scope the request. The
-        // old ApiClient sent X-Database-Id on every request; the seam must too,
-        // and the raw /query submit path is where the scope is applied.
+        // `hotdata query --database X` must scope the request: the raw /query
+        // submit path is where X-Database-Id is applied.
         let mut server = mockito::Server::new();
         let m = server
             .mock("POST", "/v1/query")

src/tables.rs

@@ -38,8 +38,8 @@ pub fn list(
 ) {
     let api = Api::new(Some(workspace_id));
 
-    // The CLI only requests columns when a connection is specified, matching
-    // the old behavior (include_columns=true iff connection_id is set).
+    // Request columns only when a connection is specified
+    // (include_columns=true iff connection_id is set).
     let include_columns = connection_id.map(|_| true);
 
     let body = crate::sdk::block(api.client().information_schema().get(