From f5d9f325c69e746f678d0a02f478f1a378c88097 Mon Sep 17 00:00:00 2001 From: damocles Date: Mon, 8 Jun 2026 19:41:31 +0200 Subject: [PATCH] refactor(#1474): replace approvals lookup 7-tuple with named struct, drop type_complexity allow --- hive-c0re/src/approvals.rs | 131 ++++++++++++++++--------------------- 1 file changed, 58 insertions(+), 73 deletions(-) diff --git a/hive-c0re/src/approvals.rs b/hive-c0re/src/approvals.rs index 436fa095..21a4b617 100644 --- a/hive-c0re/src/approvals.rs +++ b/hive-c0re/src/approvals.rs @@ -166,44 +166,20 @@ impl Approvals { /// Mark pending -> approved (or fail if not pending). Returns the (now-updated) /// approval so the caller can run the action and pass the agent name. - #[allow(clippy::type_complexity)] pub fn mark_approved(&self, id: i64) -> Result { let conn = self.conn.lock().unwrap(); - // Row shape: (agent, kind, commit_ref, requested_at, status, - // fetched_sha, description). - let current: Option<( - String, - String, - String, - i64, - String, - Option, - Option, - )> = conn + let row: Option = conn .query_row( - "SELECT agent, kind, commit_ref, requested_at, status, fetched_sha, description - FROM approvals WHERE id = ?1", + ApprovalLookup::SELECT, params![id], - |row| { - Ok(( - row.get(0)?, - row.get(1)?, - row.get(2)?, - row.get(3)?, - row.get(4)?, - row.get(5)?, - row.get(6)?, - )) - }, + ApprovalLookup::from_row, ) .optional()?; - let Some((agent, kind, commit_ref, requested_at, status, fetched_sha, description)) = - current - else { + let Some(row) = row else { bail!("approval {id} not found"); }; - if status != "pending" { - bail!("approval {id} is {status}, not pending"); + if row.status != "pending" { + bail!("approval {id} is {}, not pending", row.status); } let resolved_at = now_unix(); conn.execute( @@ -212,15 +188,15 @@ impl Approvals { )?; Ok(Approval { id, - agent, - kind: kind_from_str(&kind)?, - commit_ref, - requested_at, + agent: row.agent, + kind: kind_from_str(&row.kind)?, + commit_ref: row.commit_ref, + requested_at: row.requested_at, status: ApprovalStatus::Approved, resolved_at: Some(resolved_at), note: None, - fetched_sha, - description, + fetched_sha: row.fetched_sha, + description: row.description, }) } @@ -251,45 +227,20 @@ impl Approvals { /// kind / agent / sha. Errors if the approval isn't pending — once /// it's approved/denied/failed/cancelled, the resolution is final. pub fn mark_cancelled(&self, id: i64, canceller: &str) -> Result { - // Row-shape alias for the SELECT below so we don't trip - // clippy::type_complexity. Order matches the SELECT projection: - // agent, kind, commit_ref, requested_at, status, fetched_sha, - // description. - type CancelLookupRow = ( - String, - String, - String, - i64, - String, - Option, - Option, - ); let mut conn = self.conn.lock().unwrap(); let tx = conn.transaction()?; - let row: Option = tx + let row: Option = tx .query_row( - "SELECT agent, kind, commit_ref, requested_at, status, fetched_sha, description - FROM approvals WHERE id = ?1", + ApprovalLookup::SELECT, params![id], - |row| { - Ok(( - row.get(0)?, - row.get(1)?, - row.get(2)?, - row.get(3)?, - row.get(4)?, - row.get(5)?, - row.get(6)?, - )) - }, + ApprovalLookup::from_row, ) .optional()?; - let Some((agent, kind, commit_ref, requested_at, status, fetched_sha, description)) = row - else { + let Some(row) = row else { bail!("approval {id} not found"); }; - if status != "pending" { - bail!("approval {id} is {status}, not pending"); + if row.status != "pending" { + bail!("approval {id} is {}, not pending", row.status); } let resolved_at = now_unix(); let note = format!("cancelled by {canceller}"); @@ -300,15 +251,15 @@ impl Approvals { tx.commit()?; Ok(Approval { id, - agent, - kind: kind_from_str(&kind)?, - commit_ref, - requested_at, + agent: row.agent, + kind: kind_from_str(&row.kind)?, + commit_ref: row.commit_ref, + requested_at: row.requested_at, status: ApprovalStatus::Cancelled, resolved_at: Some(resolved_at), note: Some(note), - fetched_sha, - description, + fetched_sha: row.fetched_sha, + description: row.description, }) } @@ -325,6 +276,40 @@ impl Approvals { } } +/// Columns needed to rebuild an [`Approval`] after a status transition, +/// shared by `mark_approved` / `mark_cancelled`. Replaces a 7-field +/// tuple that tripped `clippy::type_complexity` and was duplicated +/// across both callers (one suppressed the lint, the other aliased the +/// tuple) — one named projection + mapper now backs both. +struct ApprovalLookup { + agent: String, + kind: String, + commit_ref: String, + requested_at: i64, + status: String, + fetched_sha: Option, + description: Option, +} + +impl ApprovalLookup { + /// The single-row lookup by id (`?1`). Column order matches + /// [`ApprovalLookup::from_row`]. + const SELECT: &str = "SELECT agent, kind, commit_ref, requested_at, status, fetched_sha, \ + description FROM approvals WHERE id = ?1"; + + fn from_row(row: &rusqlite::Row<'_>) -> rusqlite::Result { + Ok(Self { + agent: row.get(0)?, + kind: row.get(1)?, + commit_ref: row.get(2)?, + requested_at: row.get(3)?, + status: row.get(4)?, + fetched_sha: row.get(5)?, + description: row.get(6)?, + }) + } +} + /// Collect approval rows, dropping (and logging) any that fail to /// deserialize. A single malformed / unknown-kind row must never blank /// the whole list: `collect::>()` is all-or-nothing, so one