From 8025e48c66dd4b5a544358b4a347449d0a65273a Mon Sep 17 00:00:00 2001 From: Albert Lionelle Date: Tue, 9 Jun 2026 17:32:43 -0600 Subject: [PATCH 1/2] fix(mcp): repoint stored-degree tools at programs + auto-refresh JWT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two critical fixes from MCP tester feedback that block the stored-analysis workflow added in v0.5.0. Issue A — search_degrees / get_degree / compare_degrees returned zero rows even though import_degree had written programs. They still queried the legacy `degrees` yaml-blob table (with columns that don't exist on `programs`). Repoint all three reads at `programs`: - search_degrees gains the queryable dimensions degree_type / program_kind / discipline alongside unitid / cip_prefix / catalog_year. - get_degree precedence is now program_key (unique) -> degree_id -> natural key, and returns the lossless unified-JSON `document` for downstream tools. - compare_degrees resolves stored ids against program_key then degree_id. - Legacy `degrees` + shelved `store_degree` left untouched for back-compat. Issue C (+ B) — the MCP server cached one JWT at startup and never refreshed it, so every DB tool (IPEDS included) died ~1h in; a client re-login didn't help. DbClient now holds the full AuthState behind an Arc and calls current_token() before each request: it refreshes proactively when expired (re-reading the on-disk auth file first, so a fresh `db login` is picked up mid-process) and reactively retries once on a 401. The reqwest client also gets explicit request/connect timeouts so a stalled call fails fast instead of hanging to the 4-minute MCP ceiling (Issue B). Verified live: `db status` auto-refreshed a 6h-expired token and read succeeded; MCP search_degrees() returns all 3 imported programs and get_degree(program_key=…) returns the full document. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/mcp.md | 8 +- src/core/database/client.rs | 292 ++++++++++++++++++++++++++++----- src/mcp/server.rs | 13 +- src/mcp/tools/degrees.rs | 319 ++++++++++++++++++++++++------------ 4 files changed, 477 insertions(+), 155 deletions(-) diff --git a/docs/mcp.md b/docs/mcp.md index 18ac45f..b14f020 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -485,10 +485,10 @@ Each accepts a structured request and returns JSON. Brief inventory: | `get_institution` | Full details for one institution by UNITID | | `search_cip_codes` | Look up CIP program codes by title keyword or code prefix | | `get_lookup_codes` | Decode lookup tables (carnegie_class, award_levels, …) into label maps | -| `search_degrees` | Stored degrees filtered by UNITID / CIP / catalog year | -| `get_degree` | Fetch a single stored degree by id | -| `store_degree` | Write a degree YAML to the database (requires a logged-in user with write access) | -| `compare_degrees` | Diff two stored degrees (when both are in the database) | +| `search_degrees` | Stored programs (the normalized `programs` table) filtered by UNITID / CIP prefix / catalog year / `degree_type` / `program_kind` / `discipline` | +| `get_degree` | Fetch one stored program by `program_key` → `degree_id` → natural key; returns the lossless unified-JSON `document` | +| `import_degree` | Import a report/degree JSON into the `programs` tables (the supported write path) | +| `compare_degrees` | Diff stored programs and/or inline YAMLs side-by-side with analyze metrics | | `get_institution_completions` | Per-CIP completion counts for one institution / year, with representation ratios | | `get_completion_demographics` | Aggregate completion demographics across institutions matching filters | | `get_schools_completion_demographics` | Per-school demographics for a Carnegie / award-level cohort | diff --git a/src/core/database/client.rs b/src/core/database/client.rs index 7e90f6c..e8b9a97 100644 --- a/src/core/database/client.rs +++ b/src/core/database/client.rs @@ -16,13 +16,28 @@ //! [`super::auth::load_and_refresh`], which exchanges the saved refresh //! token for a fresh access token whenever the current one is within //! the [`AuthState::is_expired`] 60s safety buffer. - -use super::auth::{auth_file_path, load_and_refresh, load_auth_state, save_auth_state, AuthState}; +//! +//! ## Long-lived sessions (MCP server) +//! +//! The whole [`AuthState`] (access **and** refresh token) is held behind an +//! `Arc>`, and every request first calls [`DbClient::current_token`], +//! which refreshes proactively when the cached access token is expired. This +//! keeps a long-running process (the MCP server in particular) alive past the +//! 1-hour JWT expiry without a restart — and, because the refresh path re-reads +//! the on-disk auth file first, a fresh `db login` by the user is picked up +//! mid-process. A 401 from `PostgREST` triggers one reactive refresh + retry. + +use super::auth::{ + auth_file_path, load_and_refresh, load_auth_state, refresh_session, save_auth_state, AuthState, +}; use super::error::{DatabaseError, DatabaseResult}; use super::query::{FilterKind, QueryFilters}; use super::tables; use crate::core::config::DatabaseConfig; use std::path::PathBuf; +use std::sync::Arc; +use std::time::Duration; +use tokio::sync::RwLock; /// Batch size for upsert HTTP requests. const WRITE_BATCH_SIZE: usize = 500; @@ -30,6 +45,15 @@ const WRITE_BATCH_SIZE: usize = 500; /// Relative path under which `PostgREST` exposes the tables. const REST_API_PREFIX: &str = "/rest/v1"; +/// Total per-request timeout. A stalled `PostgREST` call (e.g. against a +/// half-open connection after a laptop sleep) returns a clean error well under +/// the 4-minute MCP ceiling instead of hanging the tool call. +const HTTP_TIMEOUT: Duration = Duration::from_mins(1); + +/// Connection-establishment timeout — fail fast when the endpoint is +/// unreachable rather than waiting out the full request budget. +const HTTP_CONNECT_TIMEOUT: Duration = Duration::from_secs(10); + /// Database client backed by Supabase. /// /// Wraps a single `reqwest` client and carries the credentials for every @@ -44,12 +68,14 @@ pub struct DbClient { endpoint: String, /// Project anon key — goes in the `apikey` header on every request anon_key: String, - /// Signed-in user JWT — goes in `Authorization: Bearer` on every request - user_jwt: String, + /// The signed-in session (access + refresh token + expiry), behind a lock + /// so it can be refreshed in place across a long-lived process. Cloned + /// `DbClient`s share the same session via the `Arc`. + session: Arc>, /// Path to the on-disk auth file, when known. Used to persist refreshed /// tokens back to disk so the next process startup also sees a valid - /// session. `None` when the client was constructed with an explicit JWT - /// (e.g. in tests). + /// session, and to pick up a fresh `db login` mid-process. `None` when the + /// client was constructed with an explicit JWT (e.g. in tests). auth_path: Option, } @@ -78,48 +104,117 @@ impl DbClient { .ok_or_else(|| { DatabaseError::NotAuthenticated(format!("no auth file at {}", auth_path.display())) })?; - Self::new_with_auth_path( - &config.endpoint, - &config.anon_key, - state.access_token, - Some(auth_path), - ) + Self::new_with_session(&config.endpoint, &config.anon_key, state, Some(auth_path)) } - /// Create a client with explicit credentials. Useful for tests and for - /// callers that have already loaded a session manually. + /// Create a client with an explicit JWT. Useful for tests and for callers + /// that already hold a token. The synthesised session carries no refresh + /// token and never expires, so such a client never attempts a refresh. /// /// # Errors /// /// - [`DatabaseError::NotConfigured`] if endpoint or anon key are empty. /// - [`DatabaseError::NotAuthenticated`] if `user_jwt` is empty. pub fn new(endpoint: &str, anon_key: &str, user_jwt: String) -> DatabaseResult { - Self::new_with_auth_path(endpoint, anon_key, user_jwt, None) + let state = AuthState { + access_token: user_jwt, + refresh_token: String::new(), + // Far-future expiry → `is_valid()` is always true → no refresh path. + expires_at: i64::MAX, + user_email: None, + }; + Self::new_with_session(endpoint, anon_key, state, None) } - fn new_with_auth_path( + fn new_with_session( endpoint: &str, anon_key: &str, - user_jwt: String, + state: AuthState, auth_path: Option, ) -> DatabaseResult { if endpoint.is_empty() || anon_key.is_empty() { return Err(DatabaseError::NotConfigured); } - if user_jwt.is_empty() { + if state.access_token.is_empty() { return Err(DatabaseError::NotAuthenticated( "empty user JWT".to_string(), )); } + let http = reqwest::Client::builder() + .timeout(HTTP_TIMEOUT) + .connect_timeout(HTTP_CONNECT_TIMEOUT) + .build() + .map_err(|e| { + DatabaseError::ConnectionError(format!("HTTP client build failed: {e}")) + })?; Ok(Self { - http: reqwest::Client::new(), + http, endpoint: endpoint.to_string(), anon_key: anon_key.to_string(), - user_jwt, + session: Arc::new(RwLock::new(state)), auth_path, }) } + /// Return a usable access token, refreshing proactively when the cached one + /// is expired (or within the 60s safety buffer). + /// + /// # Errors + /// [`DatabaseError::NotAuthenticated`] if a refresh was needed but failed + /// (network error, revoked refresh token, missing auth file). + async fn current_token(&self) -> DatabaseResult { + { + let session = self.session.read().await; + if session.is_valid() { + return Ok(session.access_token.clone()); + } + } + self.reauthenticate(false).await + } + + /// Refresh the in-memory session and return the new access token. + /// + /// `force` skips the "another task already refreshed" fast path so a 401 on + /// a clock-valid token still triggers a re-auth. Holds the write lock across + /// the network call so concurrent callers don't stampede the refresh. + async fn reauthenticate(&self, force: bool) -> DatabaseResult { + let mut guard = self.session.write().await; + if !force && guard.is_valid() { + return Ok(guard.access_token.clone()); + } + let fresh = self.load_fresh_state(&guard).await?; + let token = fresh.access_token.clone(); + *guard = fresh; + drop(guard); + Ok(token) + } + + /// Obtain a fresh [`AuthState`]. Prefers the on-disk auth file (so a fresh + /// `db login` is picked up without a restart) via [`load_and_refresh`], + /// which also persists the refreshed token; falls back to refreshing the + /// in-memory refresh token for clients with no tracked auth file. + async fn load_fresh_state(&self, current: &AuthState) -> DatabaseResult { + if let Some(path) = self.auth_path.as_deref() { + return load_and_refresh(path, &self.endpoint, &self.anon_key) + .await + .map_err(DatabaseError::NotAuthenticated)? + .ok_or_else(|| { + DatabaseError::NotAuthenticated(format!( + "auth file disappeared at {}; run `nuanalytics db login`", + path.display() + )) + }); + } + if current.refresh_token.is_empty() { + return Err(DatabaseError::NotAuthenticated( + "session expired and no refresh token available".to_string(), + )); + } + refresh_session(&self.endpoint, &self.anon_key, ¤t.refresh_token) + .await + .map_err(DatabaseError::NotAuthenticated) + } + /// Refresh the cached email associated with the current session, if any. /// Returns `None` when no auth file is tracked (e.g. test clients) or /// when the on-disk file no longer carries an email. @@ -182,15 +277,15 @@ impl DbClient { limit: Option, ) -> DatabaseResult { let url = build_select_url(&self.endpoint, table, select_cols, filters, limit); - let response = self - .http - .get(&url) - .header("apikey", &self.anon_key) - .header("Authorization", format!("Bearer {}", self.user_jwt)) - .header("Accept", "application/json") - .send() - .await - .map_err(|e| DatabaseError::QueryError(format!("HTTP request failed: {e}")))?; + + let mut token = self.current_token().await?; + let mut response = self.send_get(&url, &token).await?; + // A 401 here means the token was rejected despite looking valid by the + // clock (revoked, clock skew). Force one reauth + retry before giving up. + if response.status().as_u16() == 401 { + token = self.reauthenticate(true).await?; + response = self.send_get(&url, &token).await?; + } if !response.status().is_success() { let status = response.status().as_u16(); @@ -206,6 +301,39 @@ impl DbClient { .map_err(|e| DatabaseError::ParseError(e.to_string())) } + /// Issue a single authenticated `GET`. Split out so [`Self::select`] can + /// reissue it with a fresh token after a 401. + async fn send_get(&self, url: &str, token: &str) -> DatabaseResult { + self.http + .get(url) + .header("apikey", &self.anon_key) + .header("Authorization", format!("Bearer {token}")) + .header("Accept", "application/json") + .send() + .await + .map_err(|e| DatabaseError::QueryError(format!("HTTP request failed: {e}"))) + } + + /// Issue a single authenticated upsert `POST` for one chunk. Split out so + /// [`Self::upsert_batch`] can reissue it with a fresh token after a 401. + async fn send_upsert( + &self, + url: &str, + token: &str, + chunk: &[serde_json::Value], + ) -> DatabaseResult { + self.http + .post(url) + .header("apikey", &self.anon_key) + .header("Authorization", format!("Bearer {token}")) + .header("Content-Type", "application/json") + .header("Prefer", "resolution=merge-duplicates") + .json(chunk) + .send() + .await + .map_err(|e| DatabaseError::QueryError(format!("HTTP request failed: {e}"))) + } + /// Upsert a batch of records. /// /// Requires the same authenticated session as [`Self::select`]; with the @@ -248,17 +376,12 @@ impl DbClient { ); for chunk in json_records.chunks(WRITE_BATCH_SIZE) { - let response = self - .http - .post(&url) - .header("apikey", &self.anon_key) - .header("Authorization", format!("Bearer {}", self.user_jwt)) - .header("Content-Type", "application/json") - .header("Prefer", "resolution=merge-duplicates") - .json(chunk) - .send() - .await - .map_err(|e| DatabaseError::QueryError(format!("HTTP request failed: {e}")))?; + let mut token = self.current_token().await?; + let mut response = self.send_upsert(&url, &token, chunk).await?; + if response.status().as_u16() == 401 { + token = self.reauthenticate(true).await?; + response = self.send_upsert(&url, &token, chunk).await?; + } if !response.status().is_success() { let status = response.status().as_u16(); @@ -356,7 +479,16 @@ mod tests { .expect("complete credentials must produce a client"); assert_eq!(client.endpoint, "https://example.supabase.co"); assert_eq!(client.anon_key, "anon"); - assert_eq!(client.user_jwt, "jwt"); + let session = client + .session + .try_read() + .expect("uncontended session read in test"); + assert_eq!(session.access_token, "jwt"); + // The explicit-JWT constructor synthesises a non-expiring session with + // no refresh token, so such a client never attempts a refresh. + assert!(session.refresh_token.is_empty()); + assert!(session.is_valid()); + drop(session); assert!(client.auth_path().is_none()); } @@ -507,4 +639,82 @@ mod tests { other => panic!("expected NotAuthenticated, got {other:?}"), } } + + fn auth_state(access: &str, refresh: &str, expires_offset_secs: i64) -> AuthState { + AuthState { + access_token: access.to_string(), + refresh_token: refresh.to_string(), + expires_at: chrono::Utc::now().timestamp() + expires_offset_secs, + user_email: None, + } + } + + #[tokio::test] + async fn current_token_returns_cached_token_without_network_when_valid() { + // `new` synthesises a far-future expiry. Pointing at an unreachable + // endpoint proves no refresh round-trip is attempted for a valid token. + let client = DbClient::new( + "http://127.0.0.1:1/never", + "anon", + "valid-token".to_string(), + ) + .expect("test client"); + let token = client + .current_token() + .await + .expect("a valid token must not trigger a refresh"); + assert_eq!(token, "valid-token"); + } + + #[tokio::test] + async fn explicit_jwt_client_never_refreshes() { + let client = + DbClient::new("http://127.0.0.1:1/never", "anon", "tok".to_string()).expect("client"); + // Both the proactive and the non-forced reauth paths short-circuit on a + // session that is valid by the clock. + assert_eq!(client.current_token().await.unwrap(), "tok"); + assert_eq!(client.reauthenticate(false).await.unwrap(), "tok"); + } + + #[tokio::test] + async fn current_token_picks_up_fresh_disk_session_when_cached_is_expired() { + // Mirrors the field report's "user re-ran `db login` but the MCP server + // didn't notice" case: a fresh, valid session on disk must be adopted + // without a network refresh (the unreachable endpoint proves it). + let tmp = tempfile::tempdir().expect("tempdir"); + let path = tmp.path().join("auth.json"); + save_auth_state(&path, &auth_state("disk-fresh", "r", 3600)).expect("write disk session"); + + let stale = auth_state("stale", "old-refresh", -100); + let client = + DbClient::new_with_session("http://127.0.0.1:1/never", "anon", stale, Some(path)) + .expect("client"); + + let token = client + .current_token() + .await + .expect("must adopt the fresh disk session"); + assert_eq!( + token, "disk-fresh", + "a valid on-disk re-login must be picked up without a restart or network call" + ); + } + + #[tokio::test] + async fn current_token_errors_when_expired_and_no_refresh_available() { + // Expired in-memory session, no auth file, empty refresh token → there + // is nothing to refresh with, so it must fail fast rather than hang. + let expired = auth_state("x", "", -100); + let client = + DbClient::new_with_session("https://example.supabase.co", "anon", expired, None) + .expect("client"); + let err = client + .current_token() + .await + .expect_err("expired session with no refresh path must error"); + assert!( + matches!(err, DatabaseError::NotAuthenticated(_)), + "got {err:?}" + ); + } } diff --git a/src/mcp/server.rs b/src/mcp/server.rs index 870c9cf..4306dc1 100644 --- a/src/mcp/server.rs +++ b/src/mcp/server.rs @@ -508,7 +508,7 @@ impl NuAnalyticsMcpServer { /// Search stored degree programs in the database #[cfg(feature = "database")] #[tool( - description = "Search stored degree programs in the database. Filter by institution UNITID, CIP code prefix (\"11.\" for CS), or catalog year. Returns matching degrees with metadata. Use get_degree to retrieve the full YAML content." + description = "Search stored programs in the database (the normalized `programs` table written by import_degree). Filter by institution UNITID, CIP code prefix (\"11.\" for CS), catalog year, and the queryable dimensions degree_type (\"BS\"/\"BA\"/\"MS\"/\"MINOR\"), program_kind (\"major\"/\"minor\"/\"concentration\"/…), and discipline (\"cs\"/\"ai\"/\"ds\"/…). Returns matching programs with metadata (program_key, name, verified, has_impossible_requirements, …). Use get_degree to retrieve the full unified-JSON document." )] fn search_degrees(&self, Parameters(req): Parameters) -> String { self.call_db("search_degrees", |db| async move { @@ -519,7 +519,7 @@ impl NuAnalyticsMcpServer { /// Retrieve a stored degree program by ID or natural key #[cfg(feature = "database")] #[tool( - description = "Retrieve a full degree program YAML. Lookup by degree_id (fastest) or by natural key: unitid + cip_code + catalog_year. If multiple degrees match the natural key, returns a list of summaries — narrow with more filters or use degree_id. Returns full YAML content usable with validate_degree / analyze_degree." + description = "Retrieve a full stored program from the `programs` table. Lookup precedence: program_key (unique, strongest) → degree_id → natural key (unitid + cip_code + catalog_year). If multiple programs match, returns a list of summaries — narrow with program_key or more filters. On a single match, returns the program metadata plus its `document` (the lossless unified-JSON degree) directly usable with validate_degree / analyze_degree / cache_yaml." )] fn get_degree(&self, Parameters(req): Parameters) -> String { self.call_db("get_degree", |db| async move { @@ -530,7 +530,7 @@ impl NuAnalyticsMcpServer { /// Compare multiple stored degree programs (and/or inline YAMLs) #[cfg(feature = "database")] #[tool( - description = "Compare multiple degrees side-by-side. Provide `sources` (structured list of {label?, degree_id?|yaml_content?|yaml_path?}, exactly one source per entry) and/or `degree_ids` (legacy comma-separated stored-IDs form). `sources` lets you benchmark an in-progress inline YAML against a stored peer without storing it first. Returns metadata + YAML content per entry, plus (default) analyze-style metrics (complexity, longest_delay, total_credits, plans_analyzed). Set include_metrics=false to skip the analysis pass." + description = "Compare multiple degrees side-by-side. Provide `sources` (structured list of {label?, degree_id?|yaml_content?|yaml_path?}, exactly one source per entry) and/or `degree_ids` (legacy comma-separated stored-IDs form; each id is matched against program_key then degree_id in the `programs` table). `sources` lets you benchmark an in-progress inline YAML against a stored peer without storing it first. Returns per entry: program_key, name, the degree `source` text (unified JSON for stored programs, raw YAML for inline), plus (default) analyze-style metrics (complexity, longest_delay, total_credits, plans_analyzed). Set include_metrics=false to skip the analysis pass." )] fn compare_degrees(&self, Parameters(req): Parameters) -> String { self.call_db("compare_degrees", |db| async move { @@ -823,9 +823,10 @@ impl ServerHandler for NuAnalyticsMcpServer { \t→ per-school CS demographics for all R1 schools >1000 students (3 DB calls, not 130)\n\ - get_completion_demographics(carnegie_class=15, award_level=5, year=2024) → aggregate across all matched schools\n\ \n\ - Degree programs:\n\ - - search_degrees(unitid=167358) / get_degree(unitid=167358, cip_code=\"11.0101\") → retrieve stored degrees\n\ - - search_degrees + get_degree → read-only DB access; storage of new degrees is not yet provisioned" + Degree programs (normalized `programs` table):\n\ + - search_degrees(unitid=167358) / search_degrees(degree_type=\"BS\", discipline=\"cs\") → browse stored programs\n\ + - get_degree(unitid=167358, cip_code=\"11.0101\", catalog_year=\"2025-2026\") or get_degree(program_key=…) → full program + its unified-JSON document\n\ + - import_degree(json_path=…) → write a program (and optional analysis run) from a report/degree JSON" } else { "\n\nDatabase tools: Not available (database not configured or disabled)" }; diff --git a/src/mcp/tools/degrees.rs b/src/mcp/tools/degrees.rs index ea5f5fe..04ce3fa 100644 --- a/src/mcp/tools/degrees.rs +++ b/src/mcp/tools/degrees.rs @@ -1,4 +1,9 @@ //! `search_degrees`, `get_degree`, `compare_degrees`, and `store_degree` MCP tools +//! +//! `search_degrees` / `get_degree` / `compare_degrees` read the normalized +//! `programs` table written by `import_degree` (the unified-JSON `document` is +//! the lossless source of truth). The legacy `store_degree` still targets the +//! old `degrees` yaml-blob table and is retained only for back-compat. use std::sync::Arc; @@ -8,8 +13,11 @@ use crate::mcp::tools::shared::{self, error_json, parse_first, parse_json_array, use rmcp::schemars; use serde::{Deserialize, Serialize}; -const DEGREE_SUMMARY_COLS: &str = "degree_id,unitid,cip_code,catalog_year,created_at"; -const DEGREE_DETAIL_COLS: &str = "degree_id,unitid,cip_code,catalog_year,yaml_content"; +/// Lightweight projection for `search_degrees` results. +const PROGRAM_SUMMARY_COLS: &str = "program_key,name,unitid,cip_code,catalog_year,degree_type,program_kind,discipline,verified,institution_resolved,has_impossible_requirements"; +/// Full projection for `get_degree` — the summary fields plus provenance and +/// the lossless `document` (unified-JSON degree) for downstream analysis. +const PROGRAM_DETAIL_COLS: &str = "program_key,name,unitid,cip_code,catalog_year,degree_type,program_kind,discipline,verified,institution_resolved,has_impossible_requirements,degree_id,institution_raw,total_credits,source_url,document"; // ============================================================================ // Request types @@ -28,6 +36,19 @@ pub struct SearchDegreesRequest { /// Catalog year string (e.g. `\"2024-2025\"`) #[schemars(description = "Catalog year (e.g. \"2024-2025\")")] pub catalog_year: Option, + /// Normalized degree-type code (e.g. `\"BS\"`, `\"BA\"`, `\"MS\"`, `\"MINOR\"`) + #[schemars( + description = "Normalized degree type code (e.g. \"BS\", \"BA\", \"MS\", \"MINOR\")" + )] + pub degree_type: Option, + /// Program kind (e.g. `\"major\"`, `\"minor\"`, `\"concentration\"`, `\"certificate\"`) + #[schemars( + description = "Program kind (e.g. \"major\", \"minor\", \"concentration\", \"certificate\")" + )] + pub program_kind: Option, + /// Discipline tag (e.g. `\"cs\"`, `\"ai\"`, `\"ds\"`, `\"cy\"`) + #[schemars(description = "Discipline (e.g. \"cs\", \"ai\", \"ds\", \"cy\")")] + pub discipline: Option, /// Maximum results to return (default 20, max 50) #[schemars(description = "Maximum results (default 20, max 50)")] #[serde(default, deserialize_with = "shared::deserialize_opt_usize")] @@ -36,14 +57,21 @@ pub struct SearchDegreesRequest { /// Request parameters for `get_degree` /// -/// Lookup by natural key `(unitid, cip_code, catalog_year)` or by `degree_id`. -/// If multiple degrees match, returns a list of summaries — narrow with more filters. +/// Lookup precedence: `program_key` (unique) → `degree_id` → natural key +/// `(unitid, cip_code, catalog_year)`. If multiple programs match, returns a +/// list of summaries — narrow with more filters. #[derive(Debug, Deserialize, schemars::JsonSchema)] pub struct GetDegreeRequest { - /// Direct lookup by unique degree identifier (e.g. `\"neu-khoury-bscs-2024\"`). - /// Takes priority over natural-key fields if provided. + /// Deterministic program key (the strongest, unique lookup). Takes priority + /// over every other field. + #[schemars( + description = "Stored program_key (unique, strongest lookup). Takes priority over other fields." + )] + pub program_key: Option, + /// Lookup by `Degree.id` slug (e.g. `\"neu-khoury-bscs-2024\"`). May match + /// several programs across catalog years. Used when `program_key` is absent. #[schemars( - description = "Unique degree ID (fastest lookup). Takes priority over other fields." + description = "Degree id slug. May match multiple catalog years; prefer program_key when known." )] pub degree_id: Option, /// IPEDS UNITID of the institution @@ -155,23 +183,48 @@ pub struct StoreDegreeRequest { // Response types // ============================================================================ -/// Lightweight degree record for search results (no YAML content). +/// Lightweight program record for search results (no `document`). #[derive(Debug, Serialize, Deserialize)] -struct DegreeSummary { - degree_id: String, +struct ProgramSummary { + program_key: String, + name: String, unitid: Option, cip_code: Option, catalog_year: Option, - created_at: Option, + degree_type: Option, + program_kind: Option, + discipline: Option, + #[serde(default)] + verified: bool, + #[serde(default)] + institution_resolved: bool, + #[serde(default)] + has_impossible_requirements: bool, } +/// Full program record for `get_degree`, including the lossless unified-JSON +/// `document` that downstream tools (`analyze_degree`, `cache_yaml`) accept. #[derive(Debug, Serialize, Deserialize)] -struct DegreeDetail { - degree_id: String, +struct ProgramDetail { + program_key: String, + name: String, unitid: Option, cip_code: Option, catalog_year: Option, - yaml_content: String, + degree_type: Option, + program_kind: Option, + discipline: Option, + #[serde(default)] + verified: bool, + #[serde(default)] + institution_resolved: bool, + #[serde(default)] + has_impossible_requirements: bool, + degree_id: Option, + institution_raw: Option, + total_credits: Option, + source_url: Option, + document: serde_json::Value, } // ============================================================================ @@ -179,90 +232,102 @@ struct DegreeDetail { // ============================================================================ /// Execute `search_degrees` and return JSON. +/// +/// Reads the normalized `programs` table, so the new queryable dimensions +/// (`degree_type`, `program_kind`, `discipline`) filter alongside the legacy +/// `unitid` / `cip_prefix`. pub async fn execute_search_json(client: &Arc, req: SearchDegreesRequest) -> String { let limit = req.limit.unwrap_or(20).min(50); let filters = QueryFilters::new() .eq("unitid", req.unitid) .eq("catalog_year", req.catalog_year.as_deref()) + .eq("degree_type", req.degree_type.as_deref()) + .eq("program_kind", req.program_kind.as_deref()) + .eq("discipline", req.discipline.as_deref()) .starts_with("cip_code", req.cip_prefix.as_deref()); let result = match client - .select(tables::DEGREES, DEGREE_SUMMARY_COLS, &filters, Some(limit)) + .select( + tables::PROGRAMS, + PROGRAM_SUMMARY_COLS, + &filters, + Some(limit), + ) .await { Ok(v) => v, Err(e) => return error_json(e), }; - let summaries: Vec = parse_json_array(&result); + let programs: Vec = parse_json_array(&result); to_json_pretty(&serde_json::json!({ - "count": summaries.len(), - "degrees": summaries + "count": programs.len(), + "programs": programs })) } -/// Execute `get_degree` and return JSON (includes full YAML content). +/// Execute `get_degree` and return JSON (includes the full unified-JSON +/// `document`). /// -/// - If `degree_id` provided: direct fetch by ID. -/// - Otherwise: filter by `unitid`, `cip_code`, `catalog_year` (any combination). -/// - If exactly 1 result: return full YAML. -/// - If >1 results: return list of summaries with disambiguation message. +/// Lookup precedence: `program_key` (unique) → `degree_id` → natural key +/// `(unitid, cip_code, catalog_year)`. Exactly 1 match → full detail; >1 → +/// disambiguation summaries. pub async fn execute_get_json(client: &Arc, req: GetDegreeRequest) -> String { - // Direct lookup by degree_id takes priority - if let Some(ref id) = req.degree_id { - return fetch_by_id(client, id).await; - } - - // Natural-key lookup — need at least one filter - if req.unitid.is_none() && req.cip_code.is_none() && req.catalog_year.is_none() { + let filters = if let Some(pk) = req.program_key.as_deref() { + QueryFilters::new().eq("program_key", Some(pk)) + } else if let Some(id) = req.degree_id.as_deref() { + QueryFilters::new().eq("degree_id", Some(id)) + } else if req.unitid.is_some() || req.cip_code.is_some() || req.catalog_year.is_some() { + QueryFilters::new() + .eq("unitid", req.unitid) + .eq("cip_code", req.cip_code.as_deref()) + .eq("catalog_year", req.catalog_year.as_deref()) + } else { return serde_json::json!({ - "error": "Provide at least one of: degree_id, unitid, cip_code, or catalog_year", - "tip": "Use search_degrees to browse available degrees first" + "error": "Provide at least one of: program_key, degree_id, unitid, cip_code, or catalog_year", + "tip": "Use search_degrees to browse available programs first" }) .to_string(); - } - - let filters = QueryFilters::new() - .eq("unitid", req.unitid) - .eq("cip_code", req.cip_code.as_deref()) - .eq("catalog_year", req.catalog_year.as_deref()); + }; - // Fetch up to 2 to detect ambiguity without fetching everything + // Fetch up to 10 to detect ambiguity without fetching everything. let result = match client - .select(tables::DEGREES, DEGREE_DETAIL_COLS, &filters, Some(10)) + .select(tables::PROGRAMS, PROGRAM_DETAIL_COLS, &filters, Some(10)) .await { Ok(v) => v, Err(e) => return error_json(e), }; - let degrees: Vec = parse_json_array(&result); + let programs: Vec = parse_json_array(&result); - match degrees.len() { + match programs.len() { 0 => serde_json::json!({ - "error": "No degree found matching the given filters", + "error": "No program found matching the given filters", "tip": "Use search_degrees to see what is available" }) .to_string(), - 1 => to_json_pretty(°rees[0]), + 1 => to_json_pretty(&programs[0]), _ => { - // Return summaries and ask to narrow - let summaries: Vec<_> = degrees + // Return summaries and ask to narrow. + let summaries: Vec<_> = programs .iter() - .map(|d| { + .map(|p| { serde_json::json!({ - "degree_id": d.degree_id, - "unitid": d.unitid, - "cip_code": d.cip_code, - "catalog_year": d.catalog_year + "program_key": p.program_key, + "name": p.name, + "unitid": p.unitid, + "cip_code": p.cip_code, + "catalog_year": p.catalog_year, + "degree_type": p.degree_type }) }) .collect(); serde_json::json!({ - "message": "Multiple degrees match — provide degree_id or more filters to narrow", - "count": degrees.len(), + "message": "Multiple programs match — provide program_key or more filters to narrow", + "count": programs.len(), "matches": summaries }) .to_string() @@ -303,7 +368,7 @@ pub async fn execute_compare_json(client: &Arc, req: CompareDegreesReq .filter(|s| !s.is_empty()) .collect(); for id in ids { - match fetch_detail_by_id(client, id).await { + match fetch_program_detail(client, id).await { Some(detail) => resolved.push(ResolvedDegree::from_detail(None, detail)), None => not_found.push(id.to_string()), } @@ -321,14 +386,16 @@ pub async fn execute_compare_json(client: &Arc, req: CompareDegreesReq .map(|rd| { let mut record = serde_json::json!({ "label": rd.label, + "program_key": rd.program_key, + "name": rd.name, "degree_id": rd.degree_id, "unitid": rd.unitid, "cip_code": rd.cip_code, "catalog_year": rd.catalog_year, - "yaml_content": rd.yaml_content, + "source": rd.source, }); if include_metrics { - record["metrics"] = compute_compare_metrics(&rd.yaml_content, max_plans); + record["metrics"] = compute_compare_metrics(&rd.source, max_plans); } record }) @@ -342,35 +409,43 @@ pub async fn execute_compare_json(client: &Arc, req: CompareDegreesReq } /// Internal: an already-resolved degree ready to fold into the response. +/// `source` holds the degree's source text — unified JSON for a stored program, +/// raw YAML for an inline/filesystem source. struct ResolvedDegree { label: Option, + program_key: Option, + name: Option, degree_id: Option, unitid: Option, cip_code: Option, catalog_year: Option, - yaml_content: String, + source: String, } impl ResolvedDegree { - fn from_detail(label: Option, detail: DegreeDetail) -> Self { + fn from_detail(label: Option, detail: ProgramDetail) -> Self { Self { label, - degree_id: Some(detail.degree_id), + program_key: Some(detail.program_key), + name: Some(detail.name), + degree_id: detail.degree_id, unitid: detail.unitid, cip_code: detail.cip_code, catalog_year: detail.catalog_year, - yaml_content: detail.yaml_content, + source: serde_json::to_string(&detail.document).unwrap_or_default(), } } - const fn from_inline(label: Option, yaml_content: String) -> Self { + const fn from_inline(label: Option, source: String) -> Self { Self { label, + program_key: None, + name: None, degree_id: None, unitid: None, cip_code: None, catalog_year: None, - yaml_content, + source, } } } @@ -397,10 +472,10 @@ async fn resolve_source( } if let Some(id) = source.degree_id.as_deref() { - return fetch_detail_by_id(client, id) + return fetch_program_detail(client, id) .await .map(|detail| ResolvedDegree::from_detail(source.label.clone(), detail)) - .ok_or_else(|| format!("{label}: degree_id {id:?} not found in database")); + .ok_or_else(|| format!("{label}: stored id {id:?} not found in database")); } if let Some(yaml) = source.yaml_content.clone() { return Ok(ResolvedDegree::from_inline(source.label.clone(), yaml)); @@ -433,16 +508,22 @@ fn validate_source_count(source: &DegreeSource) -> Result<(), &'static str> { } } -/// Fetch a stored degree's full detail by id; returns `None` if missing or -/// the row fails to deserialize. Wraps the JSON-string helper used elsewhere -/// in this module. -async fn fetch_detail_by_id(client: &Arc, id: &str) -> Option { - let result_str = fetch_by_id(client, id).await; - let parsed: serde_json::Value = serde_json::from_str(&result_str).ok()?; - if parsed.get("error").is_some() { - return None; +/// Fetch a stored program's full detail by `id`, trying `program_key` (unique) +/// first and then `degree_id`. Returns `None` if neither matches or the row +/// fails to deserialize. +async fn fetch_program_detail(client: &Arc, id: &str) -> Option { + for col in ["program_key", "degree_id"] { + let filters = QueryFilters::new().eq(col, Some(id)); + if let Ok(value) = client + .select(tables::PROGRAMS, PROGRAM_DETAIL_COLS, &filters, Some(1)) + .await + { + if let Some(detail) = parse_first::(&value) { + return Some(detail); + } + } } - serde_json::from_value::(parsed).ok() + None } /// Run the analyze pipeline on a degree's YAML and pluck the side-by-side @@ -495,41 +576,71 @@ pub async fn execute_store_json(client: &Arc, req: StoreDegreeRequest) } } -// ============================================================================ -// Internal helpers -// ============================================================================ - -/// Fetch a single degree by its `degree_id` and return full YAML detail JSON. -/// Returns an error JSON object if not found or the query fails. -async fn fetch_by_id(client: &Arc, id: &str) -> String { - let filters = QueryFilters::new().eq("degree_id", Some(id)); - - let result = match client - .select(tables::DEGREES, DEGREE_DETAIL_COLS, &filters, Some(1)) - .await - { - Ok(v) => v, - Err(e) => return error_json(e), - }; - - let degree: Option = parse_first(&result); - - degree.map_or_else( - || { - serde_json::json!({ - "error": "Degree not found", - "degree_id": id - }) - .to_string() - }, - |d| to_json_pretty(&d), - ) -} - #[cfg(test)] mod tests { use super::*; + /// Guards the `PROGRAM_SUMMARY_COLS` → `ProgramSummary` contract: a row + /// shaped like a `PostgREST` `programs` select must deserialize cleanly. A + /// typo'd column name (the original Issue A failure mode) would break this. + #[test] + fn test_program_summary_deserializes_from_programs_row() { + let row = serde_json::json!({ + "program_key": "prog:126818|11.0701|2025-2026|BS", + "name": "Computer Science, BS", + "unitid": 126_818, + "cip_code": "11.0701", + "catalog_year": "2025-2026", + "degree_type": "BS", + "program_kind": "concentration", + "discipline": "cs", + "verified": false, + "institution_resolved": true, + "has_impossible_requirements": false + }); + let summary: ProgramSummary = + serde_json::from_value(row).expect("programs summary row must deserialize"); + assert_eq!(summary.program_key, "prog:126818|11.0701|2025-2026|BS"); + assert_eq!(summary.unitid, Some(126_818)); + assert_eq!(summary.degree_type.as_deref(), Some("BS")); + assert!(summary.institution_resolved); + } + + /// Guards the `PROGRAM_DETAIL_COLS` → `ProgramDetail` contract, including + /// the lossless `document` JSONB that downstream tools consume. Nullable + /// columns (`cip_code`, `degree_id`) must tolerate JSON `null`. + #[test] + fn test_program_detail_deserializes_with_document_and_nulls() { + let row = serde_json::json!({ + "program_key": "prog:141574||2024-2025|BS", + "name": "BS in Computer Science - General Track", + "unitid": 141_574, + "cip_code": null, + "catalog_year": "2024-2025", + "degree_type": "BS", + "program_kind": null, + "discipline": null, + "verified": false, + "institution_resolved": true, + "has_impossible_requirements": false, + "degree_id": null, + "institution_raw": "University of Hawaii at Manoa", + "total_credits": 120, + "source_url": null, + "document": { "degree": { "institution": "University of Hawaii at Manoa" } } + }); + let detail: ProgramDetail = + serde_json::from_value(row).expect("programs detail row must deserialize"); + assert_eq!(detail.cip_code, None); + assert_eq!(detail.degree_id, None); + assert_eq!(detail.total_credits, Some(120)); + assert_eq!( + detail.document["degree"]["institution"], + serde_json::json!("University of Hawaii at Manoa"), + "the lossless document must survive deserialization intact" + ); + } + #[test] fn test_compute_compare_metrics_returns_metrics_for_valid_yaml() { // Minimal valid YAML lets us assert the metrics object carries the From 307af289d758a44eb51fe8f1597b35c3d233cfb4 Mon Sep 17 00:00:00 2001 From: Albert Lionelle Date: Tue, 9 Jun 2026 17:44:21 -0600 Subject: [PATCH 2/2] fix(docs): drop private intra-doc link to DbClient::current_token The client module doc linked to `current_token`, a private method, which trips rustdoc::private_intra_doc_links under the Documentation CI job's RUSTDOCFLAGS=-D warnings. Demote it to a plain code span. Verified with a clean `cargo doc --no-deps --all-features` under RUSTFLAGS/RUSTDOCFLAGS=-D warnings. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/core/database/client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/database/client.rs b/src/core/database/client.rs index e8b9a97..b6bef0b 100644 --- a/src/core/database/client.rs +++ b/src/core/database/client.rs @@ -20,7 +20,7 @@ //! ## Long-lived sessions (MCP server) //! //! The whole [`AuthState`] (access **and** refresh token) is held behind an -//! `Arc>`, and every request first calls [`DbClient::current_token`], +//! `Arc>`, and every request first calls `current_token` (private), //! which refreshes proactively when the cached access token is expired. This //! keeps a long-running process (the MCP server in particular) alive past the //! 1-hour JWT expiry without a restart — and, because the refresh path re-reads