refactor: consolidate types, remove dead code, and fix minor bugs

Replace DayOfWeek with chrono::Weekday via extension traits, unify
RateLimitConfig into the config module, and remove the unused time
command, BannerState, and ClassDetails stub. Fix open_only query
parameter to respect false values and correct 12-hour time display.

Cargo.lock

@@ -230,6 +230,7 @@ dependencies = [
  "cookie",
  "dashmap 6.1.0",
  "dotenvy",
+ "extension-traits",
  "figment",
  "fundu",
  "futures",
@@ -806,6 +807,35 @@ dependencies = [
  "pin-project-lite",
 ]
 
+[[package]]
+name = "ext-trait"
+version = "2.0.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "b0c24fe28375ffabb5479233d60a5d99930a3983ed3aa6db66dd03b830fc41b2"
+dependencies = [
+ "ext-trait-proc_macros",
+]
+
+[[package]]
+name = "ext-trait-proc_macros"
+version = "2.0.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ad551ddce9af58215158c84e1e655b2011f6355b655c13b56d88986b14d3db98"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn 2.0.106",
+]
+
+[[package]]
+name = "extension-traits"
+version = "2.0.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e5fea67d50388b3db0e51e65815ed7293703607ff9dc50d86f93e1abcc67b572"
+dependencies = [
+ "ext-trait",
+]
+
 [[package]]
 name = "fastrand"
 version = "2.3.0"

Cargo.toml

@@ -55,6 +55,7 @@ mime_guess = { version = "2.0", optional = true }
 clap = { version = "4.5", features = ["derive"] }
 rapidhash = "4.1.0"
 yansi = "1.0.1"
+extension-traits = "2"
 
 [dev-dependencies]
 

src/app.rs

@@ -6,7 +6,6 @@ use crate::services::bot::BotService;
 use crate::services::manager::ServiceManager;
 use crate::services::web::WebService;
 use crate::state::AppState;
-use crate::web::routes::BannerState;
 use figment::value::UncasedStr;
 use figment::{Figment, providers::Env};
 use sqlx::postgres::PgPoolOptions;
@@ -21,7 +20,6 @@ pub struct App {
     db_pool: sqlx::PgPool,
     banner_api: Arc<BannerApi>,
     app_state: AppState,
-    banner_state: BannerState,
     service_manager: ServiceManager,
 }
 
@@ -73,22 +71,18 @@ impl App {
         // Create BannerApi and AppState
         let banner_api = BannerApi::new_with_config(
             config.banner_base_url.clone(),
-            config.rate_limiting.clone().into(),
+            config.rate_limiting.clone(),
         )
         .expect("Failed to create BannerApi");
 
         let banner_api_arc = Arc::new(banner_api);
         let app_state = AppState::new(banner_api_arc.clone(), db_pool.clone());
 
-        // Create BannerState for web service
-        let banner_state = BannerState {};
-
         Ok(App {
             config,
             db_pool,
             banner_api: banner_api_arc,
             app_state,
-            banner_state,
             service_manager: ServiceManager::new(),
         })
     }
@@ -97,8 +91,7 @@ impl App {
     pub fn setup_services(&mut self, services: &[ServiceName]) -> Result<(), anyhow::Error> {
         // Register enabled services with the manager
         if services.contains(&ServiceName::Web) {
-            let web_service =
+            let web_service = Box::new(WebService::new(self.config.port));
-                Box::new(WebService::new(self.config.port, self.banner_state.clone()));
             self.service_manager
                 .register_service(ServiceName::Web.as_str(), web_service);
         }

src/banner/api.rs

@@ -1,32 +1,18 @@
 //! Main Banner API client implementation.
 
-use std::{
+use std::collections::HashMap;
-    collections::{HashMap, VecDeque},
-    sync::{Arc, Mutex},
-    time::Instant,
-};
 
 use crate::banner::{
-    BannerSession, SessionPool, create_shared_rate_limiter,
+    SessionPool, create_shared_rate_limiter, errors::BannerApiError, json::parse_json_with_context,
-    errors::BannerApiError,
+    middleware::TransparentMiddleware, models::*, nonce, query::SearchQuery,
-    json::parse_json_with_context,
+    rate_limit_middleware::RateLimitMiddleware, util::user_agent,
-    middleware::TransparentMiddleware,
-    models::*,
-    nonce,
-    query::SearchQuery,
-    rate_limit_middleware::RateLimitMiddleware,
-    rate_limiter::{RateLimitConfig, SharedRateLimiter},
-    util::user_agent,
 };
+use crate::config::RateLimitingConfig;
 use anyhow::{Context, Result, anyhow};
-use cookie::Cookie;
-use dashmap::DashMap;
 use http::HeaderValue;
-use reqwest::{Client, Request, Response};
+use reqwest::Client;
 use reqwest_middleware::{ClientBuilder, ClientWithMiddleware};
-use serde_json;
+use tracing::debug;
-use tl;
-use tracing::{Level, Metadata, Span, debug, error, field::ValueSet, info, span, trace, warn};
 
 /// Main Banner API client.
 pub struct BannerApi {
@@ -39,11 +25,14 @@ pub struct BannerApi {
 impl BannerApi {
     /// Creates a new Banner API client.
     pub fn new(base_url: String) -> Result<Self> {
-        Self::new_with_config(base_url, RateLimitConfig::default())
+        Self::new_with_config(base_url, RateLimitingConfig::default())
     }
 
     /// Creates a new Banner API client with custom rate limiting configuration.
-    pub fn new_with_config(base_url: String, rate_limit_config: RateLimitConfig) -> Result<Self> {
+    pub fn new_with_config(
+        base_url: String,
+        rate_limit_config: RateLimitingConfig,
+    ) -> Result<Self> {
         let rate_limiter = create_shared_rate_limiter(Some(rate_limit_config));
 
         let http = ClientBuilder::new(
@@ -111,7 +100,7 @@ impl BannerApi {
 
         let session = self.sessions.acquire(term.parse()?).await?;
         let url = format!("{}/classSearch/{}", self.base_url, endpoint);
-        let params = self.build_list_params(search, term, offset, max_results, &session.id());
+        let params = self.build_list_params(search, term, offset, max_results, session.id());
 
         let response = self
             .http
@@ -179,7 +168,7 @@ impl BannerApi {
 
         session.touch();
 
-        let params = self.build_search_params(query, term, &session.id(), sort, sort_descending);
+        let params = self.build_search_params(query, term, session.id(), sort, sort_descending);
 
         debug!(
             term = term,
@@ -358,29 +347,4 @@ impl BannerApi {
             .data
             .and_then(|courses| courses.into_iter().next()))
     }
-
-    /// Gets course details (placeholder - needs implementation).
-    pub async fn get_course_details(&self, term: &str, crn: &str) -> Result<ClassDetails> {
-        let body = serde_json::json!({
-            "term": term,
-            "courseReferenceNumber": crn,
-            "first": "first"
-        });
-
-        let url = format!("{}/searchResults/getClassDetails", self.base_url);
-        let response = self
-            .http
-            .post(&url)
-            .json(&body)
-            .send()
-            .await
-            .context("Failed to get course details")?;
-
-        let details: ClassDetails = response
-            .json()
-            .await
-            .context("Failed to parse course details response")?;
-
-        Ok(details)
-    }
 }

src/banner/errors.rs

@@ -1,7 +1,5 @@
 //! Error types for the Banner API client.
 
-use thiserror::Error;
-
 #[derive(Debug, thiserror::Error)]
 pub enum BannerApiError {
     #[error("Banner session is invalid or expired: {0}")]

src/banner/mod.rs

@@ -1,5 +1,3 @@
-#![allow(unused_imports)]
-
 //! Banner API module for interacting with Ellucian Banner systems.
 //!
 //! This module provides functionality to:

src/banner/models/courses.rs

@@ -76,9 +76,3 @@ impl Course {
             .unwrap_or("Unknown")
     }
 }
-
-/// Class details (to be implemented)
-#[derive(Debug, Clone, Serialize, Deserialize)]
-pub struct ClassDetails {
-    // TODO: Implement based on Banner API response
-}

src/banner/models/meetings.rs

@@ -1,10 +1,40 @@
 use bitflags::{Flags, bitflags};
-use chrono::{DateTime, NaiveDate, NaiveTime, Timelike, Utc};
+use chrono::{DateTime, NaiveDate, NaiveTime, Timelike, Utc, Weekday};
+use extension_traits::extension;
 use serde::{Deserialize, Deserializer, Serialize};
-use std::{cmp::Ordering, collections::HashSet, fmt::Display, str::FromStr};
+use std::{cmp::Ordering, fmt::Display, str::FromStr};
 
 use super::terms::Term;
 
+#[extension(pub trait WeekdayExt)]
+impl Weekday {
+    /// Short two-letter representation (used for ICS generation)
+    fn to_short_string(self) -> &'static str {
+        match self {
+            Weekday::Mon => "Mo",
+            Weekday::Tue => "Tu",
+            Weekday::Wed => "We",
+            Weekday::Thu => "Th",
+            Weekday::Fri => "Fr",
+            Weekday::Sat => "Sa",
+            Weekday::Sun => "Su",
+        }
+    }
+
+    /// Full day name
+    fn to_full_string(self) -> &'static str {
+        match self {
+            Weekday::Mon => "Monday",
+            Weekday::Tue => "Tuesday",
+            Weekday::Wed => "Wednesday",
+            Weekday::Thu => "Thursday",
+            Weekday::Fri => "Friday",
+            Weekday::Sat => "Saturday",
+            Weekday::Sun => "Sunday",
+        }
+    }
+}
+
 /// Deserialize a string field into a u32
 fn deserialize_string_to_u32<'de, D>(deserializer: D) -> Result<u32, D::Error>
 where
@@ -114,69 +144,33 @@ impl MeetingDays {
     }
 }
 
+impl Ord for MeetingDays {
+    fn cmp(&self, other: &Self) -> Ordering {
+        self.bits().cmp(&other.bits())
+    }
+}
+
 impl PartialOrd for MeetingDays {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        Some(self.bits().cmp(&other.bits()))
+        Some(self.cmp(other))
     }
 }
 
-impl From<DayOfWeek> for MeetingDays {
+impl From<Weekday> for MeetingDays {
-    fn from(day: DayOfWeek) -> Self {
+    fn from(day: Weekday) -> Self {
         match day {
-            DayOfWeek::Monday => MeetingDays::Monday,
+            Weekday::Mon => MeetingDays::Monday,
-            DayOfWeek::Tuesday => MeetingDays::Tuesday,
+            Weekday::Tue => MeetingDays::Tuesday,
-            DayOfWeek::Wednesday => MeetingDays::Wednesday,
+            Weekday::Wed => MeetingDays::Wednesday,
-            DayOfWeek::Thursday => MeetingDays::Thursday,
+            Weekday::Thu => MeetingDays::Thursday,
-            DayOfWeek::Friday => MeetingDays::Friday,
+            Weekday::Fri => MeetingDays::Friday,
-            DayOfWeek::Saturday => MeetingDays::Saturday,
+            Weekday::Sat => MeetingDays::Saturday,
-            DayOfWeek::Sunday => MeetingDays::Sunday,
+            Weekday::Sun => MeetingDays::Sunday,
         }
     }
 }
 
-/// Days of the week for meeting schedules
+impl TryFrom<MeetingDays> for Weekday {
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
-pub enum DayOfWeek {
-    Monday,
-    Tuesday,
-    Wednesday,
-    Thursday,
-    Friday,
-    Saturday,
-    Sunday,
-}
-
-impl DayOfWeek {
-    /// Convert to short string representation
-    ///
-    /// Do not change these, these are used for ICS generation. Casing does not matter though.
-    pub fn to_short_string(self) -> &'static str {
-        match self {
-            DayOfWeek::Monday => "Mo",
-            DayOfWeek::Tuesday => "Tu",
-            DayOfWeek::Wednesday => "We",
-            DayOfWeek::Thursday => "Th",
-            DayOfWeek::Friday => "Fr",
-            DayOfWeek::Saturday => "Sa",
-            DayOfWeek::Sunday => "Su",
-        }
-    }
-
-    /// Convert to full string representation
-    pub fn to_full_string(self) -> &'static str {
-        match self {
-            DayOfWeek::Monday => "Monday",
-            DayOfWeek::Tuesday => "Tuesday",
-            DayOfWeek::Wednesday => "Wednesday",
-            DayOfWeek::Thursday => "Thursday",
-            DayOfWeek::Friday => "Friday",
-            DayOfWeek::Saturday => "Saturday",
-            DayOfWeek::Sunday => "Sunday",
-        }
-    }
-}
-
-impl TryFrom<MeetingDays> for DayOfWeek {
     type Error = anyhow::Error;
 
     fn try_from(days: MeetingDays) -> Result<Self, Self::Error> {
@@ -187,13 +181,13 @@ impl TryFrom<MeetingDays> for DayOfWeek {
         let count = days.into_iter().count();
         if count == 1 {
             return Ok(match days {
-                MeetingDays::Monday => DayOfWeek::Monday,
+                MeetingDays::Monday => Weekday::Mon,
-                MeetingDays::Tuesday => DayOfWeek::Tuesday,
+                MeetingDays::Tuesday => Weekday::Tue,
-                MeetingDays::Wednesday => DayOfWeek::Wednesday,
+                MeetingDays::Wednesday => Weekday::Wed,
-                MeetingDays::Thursday => DayOfWeek::Thursday,
+                MeetingDays::Thursday => Weekday::Thu,
-                MeetingDays::Friday => DayOfWeek::Friday,
+                MeetingDays::Friday => Weekday::Fri,
-                MeetingDays::Saturday => DayOfWeek::Saturday,
+                MeetingDays::Saturday => Weekday::Sat,
-                MeetingDays::Sunday => DayOfWeek::Sunday,
+                MeetingDays::Sunday => Weekday::Sun,
                 _ => unreachable!(),
             });
         }
@@ -254,7 +248,12 @@ impl TimeRange {
         let minute = time.minute();
 
         let meridiem = if hour < 12 { "AM" } else { "PM" };
-        format!("{hour}:{minute:02}{meridiem}")
+        let display_hour = match hour {
+            0 => 12,
+            13..=23 => hour - 12,
+            _ => hour,
+        };
+        format!("{display_hour}:{minute:02}{meridiem}")
     }
 
     /// Get duration in minutes
@@ -365,24 +364,32 @@ pub enum MeetingLocation {
 impl MeetingLocation {
     /// Create from raw MeetingTime data
     pub fn from_meeting_time(meeting_time: &MeetingTime) -> Self {
-        if meeting_time.campus.is_none()
+        if let (
-            || meeting_time.building.is_none()
+            Some(campus),
-            || meeting_time.building_description.is_none()
+            Some(campus_description),
-            || meeting_time.room.is_none()
+            Some(building),
-            || meeting_time.campus_description.is_none()
+            Some(building_description),
-            || meeting_time
+            Some(room),
-                .campus_description
+        ) = (
-                .eq(&Some("Internet".to_string()))
+            &meeting_time.campus,
-        {
+            &meeting_time.campus_description,
+            &meeting_time.building,
+            &meeting_time.building_description,
+            &meeting_time.room,
+        ) {
+            if campus_description == "Internet" {
                 return MeetingLocation::Online;
             }
 
             MeetingLocation::InPerson {
-            campus: meeting_time.campus.as_ref().unwrap().clone(),
+                campus: campus.clone(),
-            campus_description: meeting_time.campus_description.as_ref().unwrap().clone(),
+                campus_description: campus_description.clone(),
-            building: meeting_time.building.as_ref().unwrap().clone(),
+                building: building.clone(),
-            building_description: meeting_time.building_description.as_ref().unwrap().clone(),
+                building_description: building_description.clone(),
-            room: meeting_time.room.as_ref().unwrap().clone(),
+                room: room.clone(),
+            }
+        } else {
+            MeetingLocation::Online
         }
     }
 }
@@ -451,11 +458,11 @@ impl MeetingScheduleInfo {
         }
     }
 
-    /// Convert the meeting days bitset to a enum vector
+    /// Convert the meeting days bitset to a weekday vector
-    pub fn days_of_week(&self) -> Vec<DayOfWeek> {
+    pub fn days_of_week(&self) -> Vec<Weekday> {
         self.days
             .iter()
-            .map(|day| <MeetingDays as TryInto<DayOfWeek>>::try_into(day).unwrap())
+            .map(|day| <MeetingDays as TryInto<Weekday>>::try_into(day).unwrap())
             .collect()
     }
 
@@ -483,9 +490,9 @@ impl MeetingScheduleInfo {
             );
 
             if ambiguous {
-                |day: &DayOfWeek| day.to_short_string().to_string()
+                |day: &Weekday| day.to_short_string().to_string()
             } else {
-                |day: &DayOfWeek| day.to_short_string().chars().next().unwrap().to_string()
+                |day: &Weekday| day.to_short_string().chars().next().unwrap().to_string()
             }
         };
 
@@ -509,6 +516,19 @@ impl MeetingScheduleInfo {
         }
     }
 
+    /// Sort a slice of meeting schedule infos by start time, with stable fallback to day bits.
+    ///
+    /// Meetings with a time range sort before those without one.
+    /// Among meetings without a time range, ties break by day-of-week bits.
+    pub fn sort_by_start_time(meetings: &mut [MeetingScheduleInfo]) {
+        meetings.sort_unstable_by(|a, b| match (&a.time_range, &b.time_range) {
+            (Some(a_time), Some(b_time)) => a_time.start.cmp(&b_time.start),
+            (Some(_), None) => std::cmp::Ordering::Less,
+            (None, Some(_)) => std::cmp::Ordering::Greater,
+            (None, None) => a.days.bits().cmp(&b.days.bits()),
+        });
+    }
+
     /// Get the start and end date times for the meeting
     ///
     /// Uses the start and end times of the meeting if available, otherwise defaults to midnight (00:00:00.000).

src/banner/query.rs

@@ -191,7 +191,7 @@ impl SearchQuery {
             params.insert("txt_keywordlike".to_string(), keywords.join(" "));
         }
 
-        if self.open_only.is_some() {
+        if self.open_only == Some(true) {
             params.insert("chk_open_only".to_string(), "true".to_string());
         }
 
@@ -333,9 +333,9 @@ mod tests {
         let params = SearchQuery::new().open_only(true).to_params();
         assert_eq!(params.get("chk_open_only").unwrap(), "true");
 
-        // open_only(false) still sets the param (it's `.is_some()` check)
+        // open_only(false) should NOT set the param
         let params2 = SearchQuery::new().open_only(false).to_params();
-        assert_eq!(params2.get("chk_open_only").unwrap(), "true");
+        assert!(params2.get("chk_open_only").is_none());
     }
 
     #[test]
@@ -473,7 +473,7 @@ impl std::fmt::Display for SearchQuery {
         if let Some(ref keywords) = self.keywords {
             parts.push(format!("keywords={}", keywords.join(" ")));
         }
-        if self.open_only.is_some() {
+        if self.open_only == Some(true) {
             parts.push("openOnly=true".to_string());
         }
         if let Some(ref term_part) = self.term_part {

src/banner/rate_limiter.rs

@@ -1,5 +1,6 @@
 //! Rate limiting for Banner API requests to prevent overwhelming the server.
 
+use crate::config::RateLimitingConfig;
 use governor::{
     Quota, RateLimiter,
     clock::DefaultClock,
@@ -22,38 +23,6 @@ pub enum RequestType {
     Reset,
 }
 
-/// Rate limiter configuration for different request types
-#[derive(Debug, Clone)]
-pub struct RateLimitConfig {
-    /// Requests per minute for session operations
-    pub session_rpm: u32,
-    /// Requests per minute for search operations
-    pub search_rpm: u32,
-    /// Requests per minute for metadata operations
-    pub metadata_rpm: u32,
-    /// Requests per minute for reset operations
-    pub reset_rpm: u32,
-    /// Burst allowance (extra requests allowed in short bursts)
-    pub burst_allowance: u32,
-}
-
-impl Default for RateLimitConfig {
-    fn default() -> Self {
-        Self {
-            // Very conservative for session creation
-            session_rpm: 6, // 1 every 10 seconds
-            // Moderate for search operations
-            search_rpm: 30, // 1 every 2 seconds
-            // Moderate for metadata
-            metadata_rpm: 20, // 1 every 3 seconds
-            // Low for resets
-            reset_rpm: 10, // 1 every 6 seconds
-            // Allow small bursts
-            burst_allowance: 3,
-        }
-    }
-}
-
 /// A rate limiter that manages different request types with different limits
 pub struct BannerRateLimiter {
     session_limiter: RateLimiter<NotKeyed, InMemoryState, DefaultClock>,
@@ -64,7 +33,7 @@ pub struct BannerRateLimiter {
 
 impl BannerRateLimiter {
     /// Creates a new rate limiter with the given configuration
-    pub fn new(config: RateLimitConfig) -> Self {
+    pub fn new(config: RateLimitingConfig) -> Self {
         let session_quota = Quota::with_period(Duration::from_secs(60) / config.session_rpm)
             .unwrap()
             .allow_burst(NonZeroU32::new(config.burst_allowance).unwrap());
@@ -105,7 +74,7 @@ impl BannerRateLimiter {
 
 impl Default for BannerRateLimiter {
     fn default() -> Self {
-        Self::new(RateLimitConfig::default())
+        Self::new(RateLimitingConfig::default())
     }
 }
 
@@ -113,19 +82,6 @@ impl Default for BannerRateLimiter {
 pub type SharedRateLimiter = Arc<BannerRateLimiter>;
 
 /// Creates a new shared rate limiter with custom configuration
-pub fn create_shared_rate_limiter(config: Option<RateLimitConfig>) -> SharedRateLimiter {
+pub fn create_shared_rate_limiter(config: Option<RateLimitingConfig>) -> SharedRateLimiter {
     Arc::new(BannerRateLimiter::new(config.unwrap_or_default()))
 }
-
-/// Conversion from config module's RateLimitingConfig to this module's RateLimitConfig
-impl From<crate::config::RateLimitingConfig> for RateLimitConfig {
-    fn from(config: crate::config::RateLimitingConfig) -> Self {
-        Self {
-            session_rpm: config.session_rpm,
-            search_rpm: config.search_rpm,
-            metadata_rpm: config.metadata_rpm,
-            reset_rpm: config.reset_rpm,
-            burst_allowance: config.burst_allowance,
-        }
-    }
-}

src/banner/session.rs

@@ -11,7 +11,7 @@ use once_cell::sync::Lazy;
 use rand::distr::{Alphanumeric, SampleString};
 use reqwest_middleware::ClientWithMiddleware;
 use std::collections::{HashMap, VecDeque};
-use std::num::NonZeroU32;
+
 use std::ops::{Deref, DerefMut};
 use std::sync::Arc;
 use std::time::{Duration, Instant};
@@ -63,7 +63,7 @@ pub fn nonce() -> String {
 
 impl BannerSession {
     /// Creates a new session
-    pub async fn new(unique_session_id: &str, jsessionid: &str, ssb_cookie: &str) -> Result<Self> {
+    pub fn new(unique_session_id: &str, jsessionid: &str, ssb_cookie: &str) -> Result<Self> {
         let now = Instant::now();
 
         Ok(Self {
@@ -76,8 +76,8 @@ impl BannerSession {
     }
 
     /// Returns the unique session ID
-    pub fn id(&self) -> String {
+    pub fn id(&self) -> &str {
-        self.unique_session_id.clone()
+        &self.unique_session_id
     }
 
     /// Updates the last activity timestamp
@@ -312,16 +312,12 @@ impl SessionPool {
             })
             .collect::<HashMap<String, String>>();
 
-        if !cookies.contains_key("JSESSIONID") || !cookies.contains_key("SSB_COOKIE") {
-            return Err(anyhow::anyhow!("Failed to get cookies"));
-        }
-
         let jsessionid = cookies
             .get("JSESSIONID")
-            .ok_or_else(|| anyhow::anyhow!("JSESSIONID cookie missing after validation"))?;
+            .ok_or_else(|| anyhow::anyhow!("JSESSIONID cookie missing"))?;
         let ssb_cookie = cookies
             .get("SSB_COOKIE")
-            .ok_or_else(|| anyhow::anyhow!("SSB_COOKIE cookie missing after validation"))?;
+            .ok_or_else(|| anyhow::anyhow!("SSB_COOKIE cookie missing"))?;
         let cookie_header = format!("JSESSIONID={}; SSB_COOKIE={}", jsessionid, ssb_cookie);
 
         self.http
@@ -340,7 +336,7 @@ impl SessionPool {
             .await?
             .error_for_status()
             .context("Failed to get term selection page")?;
-        // TOOD: Validate success
+        // TODO: Validate success
 
         let terms = self.get_terms("", 1, 10).await?;
         if !terms.iter().any(|t| t.code == term.to_string()) {
@@ -359,7 +355,7 @@ impl SessionPool {
         self.select_term(&term.to_string(), &unique_session_id, &cookie_header)
             .await?;
 
-        BannerSession::new(&unique_session_id, jsessionid, ssb_cookie).await
+        BannerSession::new(&unique_session_id, jsessionid, ssb_cookie)
     }
 
     /// Retrieves a list of terms from the Banner API.

src/bin/search.rs

@@ -33,8 +33,7 @@ async fn main() -> Result<()> {
     );
 
     // Create Banner API client
-    let banner_api =
+    let banner_api = BannerApi::new_with_config(config.banner_base_url, config.rate_limiting)
-        BannerApi::new_with_config(config.banner_base_url, config.rate_limiting.into())
         .expect("Failed to create BannerApi");
 
     // Get current term

src/bot/commands/gcal.rs

@@ -1,8 +1,8 @@
 //! Google Calendar command implementation.
 
-use crate::banner::{Course, DayOfWeek, MeetingScheduleInfo};
+use crate::banner::{Course, MeetingScheduleInfo};
 use crate::bot::{Context, Error, utils};
-use chrono::NaiveDate;
+use chrono::{NaiveDate, Weekday};
 use std::collections::HashMap;
 use tracing::info;
 use url::Url;
@@ -39,25 +39,18 @@ pub async fn gcal(
         1.. => {
             // Sort meeting times by start time of their TimeRange
             let mut sorted_meeting_times = meeting_times.to_vec();
-            sorted_meeting_times.sort_unstable_by(|a, b| {
+            MeetingScheduleInfo::sort_by_start_time(&mut sorted_meeting_times);
-                // Primary sort: by start time
-                match (&a.time_range, &b.time_range) {
-                    (Some(a_time), Some(b_time)) => a_time.start.cmp(&b_time.start),
-                    (Some(_), None) => std::cmp::Ordering::Less,
-                    (None, Some(_)) => std::cmp::Ordering::Greater,
-                    (None, None) => a.days.bits().cmp(&b.days.bits()),
-                }
-            });
 
             let links = sorted_meeting_times
                 .iter()
                 .map(|m| {
                     let link = generate_gcal_url(&course, m)?;
+                    let days = m.days_string().unwrap_or_else(|| "TBA".to_string());
                     let detail = match &m.time_range {
                         Some(range) => {
-                            format!("{} {}", m.days_string().unwrap(), range.format_12hr())
+                            format!("{days} {}", range.format_12hr())
                         }
-                        None => m.days_string().unwrap(),
+                        None => days,
                     };
                     Ok(LinkDetail { link, detail })
                 })
@@ -105,7 +98,9 @@ fn generate_gcal_url(
         "CRN: {}\nInstructor: {}\nDays: {}",
         course.course_reference_number,
         instructor_name,
-        meeting_time.days_string().unwrap()
+        meeting_time
+            .days_string()
+            .unwrap_or_else(|| "TBA".to_string())
     );
 
     // The event location
@@ -133,13 +128,13 @@ fn generate_rrule(meeting_time: &MeetingScheduleInfo, end_date: NaiveDate) -> St
     let by_day = days_of_week
         .iter()
         .map(|day| match day {
-            DayOfWeek::Monday => "MO",
+            Weekday::Mon => "MO",
-            DayOfWeek::Tuesday => "TU",
+            Weekday::Tue => "TU",
-            DayOfWeek::Wednesday => "WE",
+            Weekday::Wed => "WE",
-            DayOfWeek::Thursday => "TH",
+            Weekday::Thu => "TH",
-            DayOfWeek::Friday => "FR",
+            Weekday::Fri => "FR",
-            DayOfWeek::Saturday => "SA",
+            Weekday::Sat => "SA",
-            DayOfWeek::Sunday => "SU",
+            Weekday::Sun => "SU",
         })
         .collect::<Vec<&str>>()
         .join(",");

src/bot/commands/ics.rs

@@ -1,6 +1,6 @@
 //! ICS command implementation for generating calendar files.
 
-use crate::banner::{Course, MeetingScheduleInfo};
+use crate::banner::{Course, MeetingDays, MeetingScheduleInfo, WeekdayExt};
 use crate::bot::{Context, Error, utils};
 use chrono::{Datelike, NaiveDate, Utc};
 use serenity::all::CreateAttachment;
@@ -61,7 +61,14 @@ impl Holiday {
     }
 }
 
-/// University holidays that should be excluded from class schedules
+/// University holidays excluded from class schedules.
+///
+/// WARNING: These dates are specific to the UTSA 2024-2025 academic calendar and must be
+/// updated each academic year. Many of these holidays fall on different dates annually
+/// (e.g., Labor Day is the first Monday of September, Thanksgiving is the fourth Thursday
+/// of November). Ideally these would be loaded from a configuration file or computed
+/// dynamically from federal/university calendar rules.
+// TODO: Load holiday dates from configuration or compute dynamically per academic year.
 const UNIVERSITY_HOLIDAYS: &[(&str, Holiday)] = &[
     ("Labor Day", Holiday::Single { month: 9, day: 1 }),
     (
@@ -132,12 +139,7 @@ pub async fn ics(
 
     // Sort meeting times by start time
     let mut sorted_meeting_times = meeting_times.to_vec();
-    sorted_meeting_times.sort_unstable_by(|a, b| match (&a.time_range, &b.time_range) {
+    MeetingScheduleInfo::sort_by_start_time(&mut sorted_meeting_times);
-        (Some(a_time), Some(b_time)) => a_time.start.cmp(&b_time.start),
-        (Some(_), None) => std::cmp::Ordering::Less,
-        (None, Some(_)) => std::cmp::Ordering::Greater,
-        (None, None) => a.days.bits().cmp(&b.days.bits()),
-    });
 
     // Generate ICS content
     let (ics_content, excluded_holidays) =
@@ -352,26 +354,10 @@ fn generate_event_content(
     Ok((event_content, Vec::new()))
 }
 
-/// Convert chrono::Weekday to the custom DayOfWeek enum
-fn chrono_weekday_to_day_of_week(weekday: chrono::Weekday) -> crate::banner::meetings::DayOfWeek {
-    use crate::banner::meetings::DayOfWeek;
-    match weekday {
-        chrono::Weekday::Mon => DayOfWeek::Monday,
-        chrono::Weekday::Tue => DayOfWeek::Tuesday,
-        chrono::Weekday::Wed => DayOfWeek::Wednesday,
-        chrono::Weekday::Thu => DayOfWeek::Thursday,
-        chrono::Weekday::Fri => DayOfWeek::Friday,
-        chrono::Weekday::Sat => DayOfWeek::Saturday,
-        chrono::Weekday::Sun => DayOfWeek::Sunday,
-    }
-}
-
 /// Check if a class meets on a specific date based on its meeting days
 fn class_meets_on_date(meeting_time: &MeetingScheduleInfo, date: NaiveDate) -> bool {
-    let weekday = chrono_weekday_to_day_of_week(date.weekday());
+    let day: MeetingDays = date.weekday().into();
-    let meeting_days = meeting_time.days_of_week();
+    meeting_time.days.contains(day)
-
-    meeting_days.contains(&weekday)
 }
 
 /// Get holiday dates that fall within the course date range and would conflict with class meetings

src/bot/commands/mod.rs

@@ -4,10 +4,8 @@ pub mod gcal;
 pub mod ics;
 pub mod search;
 pub mod terms;
-pub mod time;
 
 pub use gcal::gcal;
 pub use ics::ics;
 pub use search::search;
 pub use terms::terms;
-pub use time::time;

src/bot/commands/search.rs

@@ -4,8 +4,12 @@ use crate::banner::{SearchQuery, Term};
 use crate::bot::{Context, Error};
 use anyhow::anyhow;
 use regex::Regex;
+use std::sync::LazyLock;
 use tracing::info;
 
+static RANGE_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"(\d{1,4})-(\d{1,4})?").unwrap());
+static WILDCARD_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"(\d+)(x+)").unwrap());
+
 /// Search for courses with various filters
 #[poise::command(slash_command, prefix_command)]
 pub async fn search(
@@ -82,8 +86,7 @@ fn parse_course_code(input: &str) -> Result<(i32, i32), Error> {
 
     // Handle range format (e.g, "3000-3999")
     if input.contains('-') {
-        let re = Regex::new(r"(\d{1,4})-(\d{1,4})?").unwrap();
+        if let Some(captures) = RANGE_RE.captures(input) {
-        if let Some(captures) = re.captures(input) {
             let low: i32 = captures[1].parse()?;
             let high = if captures.get(2).is_some() {
                 captures[2].parse()?
@@ -110,8 +113,7 @@ fn parse_course_code(input: &str) -> Result<(i32, i32), Error> {
             return Err(anyhow!("Wildcard format must be exactly 4 characters"));
         }
 
-        let re = Regex::new(r"(\d+)(x+)").unwrap();
+        if let Some(captures) = WILDCARD_RE.captures(input) {
-        if let Some(captures) = re.captures(input) {
             let prefix: i32 = captures[1].parse()?;
             let x_count = captures[2].len();
 

src/bot/commands/time.rs

@@ -1,25 +0,0 @@
-//! Time command implementation for course meeting times.
-
-use crate::bot::{Context, Error, utils};
-use tracing::info;
-
-/// Get meeting times for a specific course
-#[poise::command(slash_command, prefix_command)]
-pub async fn time(
-    ctx: Context<'_>,
-    #[description = "Course Reference Number (CRN)"] crn: i32,
-) -> Result<(), Error> {
-    ctx.defer().await?;
-
-    let course = utils::get_course_by_crn(&ctx, crn).await?;
-
-    // TODO: Implement actual meeting time retrieval and display
-    ctx.say(format!(
-        "Meeting time display for '{}' is not yet implemented.",
-        course.display_title()
-    ))
-    .await?;
-
-    info!(crn = %crn, "time command completed");
-    Ok(())
-}

src/bot/mod.rs

@@ -14,7 +14,6 @@ pub fn get_commands() -> Vec<poise::Command<Data, Error>> {
     vec![
         commands::search(),
         commands::terms(),
-        commands::time(),
         commands::ics(),
         commands::gcal(),
     ]

src/config/mod.rs

@@ -70,7 +70,7 @@ fn default_banner_base_url() -> String {
 }
 
 /// Rate limiting configuration for Banner API requests
-#[derive(Deserialize, Clone, Debug)]
+#[derive(Deserialize, Clone, Debug, PartialEq, Eq)]
 pub struct RateLimitingConfig {
     /// Requests per minute for session operations (very conservative)
     #[serde(default = "default_session_rpm")]
@@ -91,13 +91,19 @@ pub struct RateLimitingConfig {
 
 /// Default rate limiting configuration
 fn default_rate_limiting() -> RateLimitingConfig {
-    RateLimitingConfig {
+    RateLimitingConfig::default()
+}
+
+impl Default for RateLimitingConfig {
+    fn default() -> Self {
+        Self {
             session_rpm: default_session_rpm(),
             search_rpm: default_search_rpm(),
             metadata_rpm: default_metadata_rpm(),
             reset_rpm: default_reset_rpm(),
             burst_allowance: default_burst_allowance(),
         }
+    }
 }
 
 /// Default session requests per minute (6 = 1 every 10 seconds)

src/formatter.rs

@@ -13,12 +13,6 @@ use tracing_subscriber::registry::LookupSpan;
 use yansi::Paint;
 
 /// Cached format description for timestamps
-/// Uses 3 subsecond digits on Emscripten, 5 otherwise for better performance
-#[cfg(target_os = "emscripten")]
-const TIMESTAMP_FORMAT: &[FormatItem<'static>] =
-    format_description!("[hour]:[minute]:[second].[subsecond digits:3]");
-
-#[cfg(not(target_os = "emscripten"))]
 const TIMESTAMP_FORMAT: &[FormatItem<'static>] =
     format_description!("[hour]:[minute]:[second].[subsecond digits:5]");
 

src/services/bot.rs

@@ -125,7 +125,13 @@ impl BotService {
                 tokio::select! {
                     _ = interval.tick() => {
                         // Get the course count, update the activity if it has changed/hasn't been set this session
-                        let course_count = app_state.get_course_count().await.unwrap();
+                        let course_count = match app_state.get_course_count().await {
+                            Ok(count) => count,
+                            Err(e) => {
+                                warn!(error = %e, "Failed to fetch course count for status update");
+                                continue;
+                            }
+                        };
                         if previous_course_count.is_none() || previous_course_count != Some(course_count) {
                             ctx.set_activity(Some(ActivityData::playing(format!(
                                 "Querying {:} classes",

src/services/web.rs

@@ -1,5 +1,5 @@
 use super::Service;
-use crate::web::{BannerState, create_router};
+use crate::web::create_router;
 use std::net::SocketAddr;
 use tokio::net::TcpListener;
 use tokio::sync::broadcast;
@@ -8,15 +8,13 @@ use tracing::{info, trace, warn};
 /// Web server service implementation
 pub struct WebService {
     port: u16,
-    banner_state: BannerState,
     shutdown_tx: Option<broadcast::Sender<()>>,
 }
 
 impl WebService {
-    pub fn new(port: u16, banner_state: BannerState) -> Self {
+    pub fn new(port: u16) -> Self {
         Self {
             port,
-            banner_state,
             shutdown_tx: None,
         }
     }
@@ -30,7 +28,7 @@ impl Service for WebService {
 
     async fn run(&mut self) -> Result<(), anyhow::Error> {
         // Create the main router with Banner API routes
-        let app = create_router(self.banner_state.clone());
+        let app = create_router();
 
         let addr = SocketAddr::from(([0, 0, 0, 0], self.port));
 

src/utils/shutdown.rs

@@ -16,17 +16,3 @@ pub async fn join_tasks(handles: Vec<JoinHandle<()>>) -> Result<(), anyhow::Erro
         Ok(())
     }
 }
-
-/// Helper for joining multiple task handles with a timeout.
-///
-/// Waits for all tasks to complete within the specified timeout.
-/// If timeout occurs, remaining tasks are aborted.
-pub async fn join_tasks_with_timeout(
-    handles: Vec<JoinHandle<()>>,
-    timeout: std::time::Duration,
-) -> Result<(), anyhow::Error> {
-    match tokio::time::timeout(timeout, join_tasks(handles)).await {
-        Ok(result) => result,
-        Err(_) => Err(anyhow::anyhow!("Task join timed out after {:?}", timeout)),
-    }
-}

src/web/assets.rs

@@ -58,8 +58,7 @@ impl AssetMetadata {
 
         // ETags generated from u64 hex should be 16 characters
         etag.len() == 16
-        // Parse the hexadecimal, compare if it matches
+            && u64::from_str_radix(etag, 16)
-            && etag.parse::<u64>()
                 .map(|parsed| parsed == self.hash.0)
                 .unwrap_or(false)
     }

src/web/routes.rs

@@ -3,7 +3,7 @@
 use axum::{
     Router,
     body::Body,
-    extract::{Request, State},
+    extract::Request,
     response::{Json, Response},
     routing::get,
 };
@@ -20,7 +20,7 @@ use std::{collections::BTreeMap, time::Duration};
 #[cfg(not(feature = "embed-assets"))]
 use tower_http::cors::{Any, CorsLayer};
 use tower_http::{classify::ServerErrorsFailureClass, timeout::TimeoutLayer, trace::TraceLayer};
-use tracing::{Span, debug, info, warn};
+use tracing::{Span, debug, trace, warn};
 
 #[cfg(feature = "embed-assets")]
 use crate::web::assets::{WebAssets, get_asset_metadata_cached};
@@ -62,17 +62,12 @@ fn set_caching_headers(response: &mut Response, path: &str, etag: &str) {
     }
 }
 
-/// Shared application state for web server
-#[derive(Clone)]
-pub struct BannerState {}
-
 /// Creates the web server router
-pub fn create_router(state: BannerState) -> Router {
+pub fn create_router() -> Router {
     let api_router = Router::new()
         .route("/health", get(health))
         .route("/status", get(status))
-        .route("/metrics", get(metrics))
+        .route("/metrics", get(metrics));
-        .with_state(state);
 
     let mut router = Router::new().nest("/api", api_router);
 
@@ -215,7 +210,7 @@ async fn handle_spa_fallback_with_headers(uri: Uri, request_headers: HeaderMap)
 
 /// Health check endpoint
 async fn health() -> Json<Value> {
-    info!("health check requested");
+    trace!("health check requested");
     Json(json!({
         "status": "healthy",
         "timestamp": chrono::Utc::now().to_rfc3339()
@@ -246,7 +241,7 @@ struct StatusResponse {
 }
 
 /// Status endpoint showing bot and system status
-async fn status(State(_state): State<BannerState>) -> Json<StatusResponse> {
+async fn status() -> Json<StatusResponse> {
     let mut services = BTreeMap::new();
 
     // Bot service status - hardcoded as disabled for now
@@ -297,7 +292,7 @@ async fn status(State(_state): State<BannerState>) -> Json<StatusResponse> {
 }
 
 /// Metrics endpoint for monitoring
-async fn metrics(State(_state): State<BannerState>) -> Json<Value> {
+async fn metrics() -> Json<Value> {
     // For now, return basic metrics structure
     Json(json!({
         "banner_api": {

web/src/main.tsx

@@ -1,14 +1,11 @@
 import { StrictMode } from "react";
 import ReactDOM from "react-dom/client";
 import { RouterProvider, createRouter } from "@tanstack/react-router";
-import { ThemeProvider } from "next-themes";
-import { Theme } from "@radix-ui/themes";
 
 // Import the generated route tree
 import { routeTree } from "./routeTree.gen";
 
 import "./styles.css";
-import reportWebVitals from "./reportWebVitals.ts";
 
 // Create a new router instance
 const router = createRouter({
@@ -33,21 +30,7 @@ if (rootElement && !rootElement.innerHTML) {
   const root = ReactDOM.createRoot(rootElement);
   root.render(
     <StrictMode>
-      <ThemeProvider
-        attribute="class"
-        defaultTheme="system"
-        enableSystem
-        disableTransitionOnChange={false}
-      >
-        <Theme>
       <RouterProvider router={router} />
-        </Theme>
-      </ThemeProvider>
     </StrictMode>
   );
 }
-
-// If you want to start measuring performance in your app, pass a function
-// to log results (for example: reportWebVitals(console.log))
-// or send to an analytics endpoint. Learn more: https://bit.ly/CRA-vitals
-reportWebVitals();

web/src/reportWebVitals.ts

@@ -1,13 +0,0 @@
-const reportWebVitals = (onPerfEntry?: () => void) => {
-  if (onPerfEntry && onPerfEntry instanceof Function) {
-    void import("web-vitals").then(({ onCLS, onINP, onFCP, onLCP, onTTFB }) => {
-      onCLS(onPerfEntry);
-      onINP(onPerfEntry);
-      onFCP(onPerfEntry);
-      onLCP(onPerfEntry);
-      onTTFB(onPerfEntry);
-    });
-  }
-};
-
-export default reportWebVitals;