Skip to content

refactor(store): single module for review-history SQL #585

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ajianaz merged 1 commit into from
Oct 8, 2026
Merged

refactor(store): single module for review-history SQL #585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
230 changes: 54 additions & 176 deletions src/commands/findings.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
use anyhow::Result;
use colored::Colorize;

use crate::engine::review_store::{self, FindingFilter, FindingStats, ReviewStore, Transition};

/// Exit codes.
const EXIT_OK: i32 = 0;
const EXIT_NOT_FOUND: i32 = 1;
Expand Down Expand Up @@ -63,104 +65,50 @@ pub fn execute_findings(action: &FindingsAction) -> Result<i32> {
// Write actions (dismiss, reopen) use a read-write connection.
match action {
FindingsAction::List { .. } | FindingsAction::Stats { .. } => {
let conn = match crate::engine::db_writer::open_db_for_read() {
Some(c) => c,
None => {
eprintln!("{}", "Error: could not open cora.db".red());
return Ok(EXIT_NOT_FOUND);
}
let Ok(conn) = review_store::open_read() else {
eprintln!("{}", "Error: could not open cora.db".red());
return Ok(EXIT_NOT_FOUND);
};
let store = ReviewStore::new(&conn);
match action {
FindingsAction::List {
all,
severity,
file,
json,
limit,
} => list_findings(&conn, *all, severity, file, *json, *limit),
FindingsAction::Stats { json } => stats(&conn, *json),
} => {
let filter = FindingFilter {
all: *all,
severity: severity.clone(),
file: file.clone(),
limit: *limit,
};
list_findings(&store, &filter, *json)
}
FindingsAction::Stats { json } => stats(&store, *json),
_ => unreachable!(),
}
}
FindingsAction::Dismiss { id, reason } => {
let conn = match crate::engine::db_writer::open_db_for_write() {
Some(c) => c,
None => {
eprintln!("{}", "Error: could not open cora.db for writing".red());
return Ok(EXIT_NOT_FOUND);
}
let Ok(conn) = review_store::open_write() else {
eprintln!("{}", "Error: could not open cora.db for writing".red());
return Ok(EXIT_NOT_FOUND);
};
dismiss(&conn, *id, reason)
dismiss(&ReviewStore::new(&conn), *id, reason.as_deref())
}
FindingsAction::Reopen { id } => {
let conn = match crate::engine::db_writer::open_db_for_write() {
Some(c) => c,
None => {
eprintln!("{}", "Error: could not open cora.db for writing".red());
return Ok(EXIT_NOT_FOUND);
}
let Ok(conn) = review_store::open_write() else {
eprintln!("{}", "Error: could not open cora.db for writing".red());
return Ok(EXIT_NOT_FOUND);
};
reopen(&conn, *id)
reopen(&ReviewStore::new(&conn), *id)
}
}
}

fn list_findings(
conn: &rusqlite::Connection,
all: bool,
severity: &Option<String>,
file: &Option<String>,
json: bool,
limit: usize,
) -> Result<i32> {
let mut sql = String::from(
"SELECT f.id, f.severity, f.file_path, f.line_number, f.title, f.status,
f.fingerprint, r.created_at
FROM findings f
JOIN reviews r ON f.review_id = r.id",
);

// Build WHERE clause with parameterized placeholders to prevent SQL injection.
let mut wheres: Vec<&str> = Vec::new();
let mut params: Vec<Box<dyn rusqlite::ToSql>> = Vec::new();

if !all {
wheres.push("f.status = 'open'");
}
if let Some(s) = severity {
wheres.push("f.severity = ?");
params.push(Box::new(s.to_uppercase()));
}
if let Some(f) = file {
wheres.push("f.file_path LIKE ?");
params.push(Box::new(format!("%{f}%")));
}

if !wheres.is_empty() {
sql.push_str(" WHERE ");
sql.push_str(&wheres.join(" AND "));
}
sql.push_str(" ORDER BY f.id DESC LIMIT ?");
params.push(Box::new(limit as i64));

let param_refs: Vec<&dyn rusqlite::ToSql> = params.iter().map(|p| p.as_ref()).collect();
let mut stmt = conn.prepare(&sql)?;
let rows: Vec<ListRow> = stmt
.query(param_refs.as_slice())?
.mapped(|r| {
Ok(ListRow {
id: r.get(0)?,
severity: r.get(1)?,
file_path: r.get(2)?,
line_number: r.get(3)?,
title: r.get(4)?,
status: r.get(5)?,
fingerprint: r.get(6)?,
created_at: r.get(7)?,
})
})
.filter_map(|r| r.ok())
.collect();
fn list_findings(store: &ReviewStore<'_>, filter: &FindingFilter, json: bool) -> Result<i32> {
let rows = store.list_findings(filter)?;

if json {
println!("{}", serde_json::to_string_pretty(&rows)?);
Expand Down Expand Up @@ -208,38 +156,14 @@ fn list_findings(
Ok(EXIT_OK)
}

fn stats(conn: &rusqlite::Connection, json: bool) -> Result<i32> {
let total: i64 = conn
.query_row("SELECT count(*) FROM findings", [], |r| r.get(0))
.unwrap_or(0);

let open: i64 = conn
.query_row(
"SELECT count(*) FROM findings WHERE status = 'open'",
[],
|r| r.get(0),
)
.unwrap_or(0);

let resolved: i64 = conn
.query_row(
"SELECT count(*) FROM findings WHERE status = 'resolved'",
[],
|r| r.get(0),
)
.unwrap_or(0);

let dismissed: i64 = conn
.query_row(
"SELECT count(*) FROM findings WHERE status = 'dismissed'",
[],
|r| r.get(0),
)
.unwrap_or(0);

let reviews: i64 = conn
.query_row("SELECT count(*) FROM reviews", [], |r| r.get(0))
.unwrap_or(0);
fn stats(store: &ReviewStore<'_>, json: bool) -> Result<i32> {
let FindingStats {
total,
open,
resolved,
dismissed,
reviews,
} = store.stats()?;

if json {
let stats = serde_json::json!({
Expand Down Expand Up @@ -269,78 +193,32 @@ fn stats(conn: &rusqlite::Connection, json: bool) -> Result<i32> {
Ok(EXIT_OK)
}

fn dismiss(conn: &rusqlite::Connection, id: i64, reason: &Option<String>) -> Result<i32> {
let exists: bool = conn
.query_row(
"SELECT status FROM findings WHERE id = ?1",
rusqlite::params![id],
|r| r.get::<_, String>(0),
)
.is_ok();

if !exists {
eprintln!("{}", format!("Finding #{} not found.", id).red());
return Ok(EXIT_NOT_FOUND);
fn dismiss(store: &ReviewStore<'_>, id: i64, reason: Option<&str>) -> Result<i32> {
match store.dismiss(id, reason)? {
Transition::NotFound => {
eprintln!("{}", format!("Finding #{} not found.", id).red());
Ok(EXIT_NOT_FOUND)
}
_ => {
println!("{} Finding #{} dismissed.", "✓".green(), id);
Ok(EXIT_OK)
}
}

conn.execute(
"UPDATE findings SET status = 'dismissed' WHERE id = ?1",
rusqlite::params![id],
)?;

let note = reason.as_deref().unwrap_or("Manually dismissed via CLI");
conn.execute(
"INSERT INTO finding_events (finding_id, event_type, note) VALUES (?1, 'dismissed', ?2)",
rusqlite::params![id, note],
)?;

println!("{} Finding #{} dismissed.", "✓".green(), id);
Ok(EXIT_OK)
}

fn reopen(conn: &rusqlite::Connection, id: i64) -> Result<i32> {
let status: Option<String> = conn
.query_row(
"SELECT status FROM findings WHERE id = ?1",
rusqlite::params![id],
|r| r.get(0),
)
.ok();

match status.as_deref() {
Some("open") => {
fn reopen(store: &ReviewStore<'_>, id: i64) -> Result<i32> {
match store.reopen(id)? {
Transition::Unchanged => {
println!("{}", format!("Finding #{} is already open.", id).yellow());
return Ok(EXIT_OK);
Ok(EXIT_OK)
}
None => {
Transition::NotFound => {
eprintln!("{}", format!("Finding #{} not found.", id).red());
return Ok(EXIT_NOT_FOUND);
Ok(EXIT_NOT_FOUND)
}
Transition::Applied => {
println!("{} Finding #{} reopened.", "✓".green(), id);
Ok(EXIT_OK)
}
_ => {}
}

conn.execute(
"UPDATE findings SET status = 'open' WHERE id = ?1",
rusqlite::params![id],
)?;

conn.execute(
"INSERT INTO finding_events (finding_id, event_type, note) VALUES (?1, 'reopened', 'Manually reopened via CLI')",
rusqlite::params![id],
)?;

println!("{} Finding #{} reopened.", "✓".green(), id);
Ok(EXIT_OK)
}

#[derive(serde::Serialize)]
struct ListRow {
id: i64,
severity: String,
file_path: String,
line_number: Option<i64>,
title: String,
status: String,
fingerprint: Option<String>,
created_at: String,
}
38 changes: 7 additions & 31 deletions src/commands/review.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use tracing::debug;
use crate::config::schema::Config;
use crate::engine::Severity;
use crate::engine::chunker;
use crate::engine::db_writer;
use crate::engine::quality_gate;
use crate::engine::review_store;
use crate::engine::types::ReviewResponse;
use crate::formatters::{OutputFormat, formatter_for};
use crate::git;
Expand Down Expand Up @@ -317,7 +317,7 @@ pub async fn execute_review(
let cwd = std::env::current_dir()
.map(|p| p.to_string_lossy().to_string())
.unwrap_or_default();
let record = db_writer::ReviewRecord {
let record = review_store::ReviewRecord {
command: cmd,
project_root: &cwd,
commit_hash: commit.as_deref(),
Expand All @@ -330,20 +330,8 @@ pub async fn execute_review(
tokens,
issues: &filtered_response.issues,
};
if db_writer::save_review_to_db(&record).is_none() {
debug!("Failed to save review to cora.db");
}

// Auto-resolve findings that no longer appear in this review.
let fps: Vec<String> = filtered_response
.issues
.iter()
.map(db_writer::compute_fingerprint_pub)
.collect();
let resolved = db_writer::resolve_stale_findings(&cwd, &fps);
if resolved > 0 {
debug!(resolved, "auto-resolved stale findings");
}
// Best-effort: a history-write failure never fails the run.
review_store::persist_review_best_effort(&record);
}
let exit_code = if gate_result
.as_ref()
Expand Down Expand Up @@ -717,7 +705,7 @@ async fn execute_chunked_review(
let cwd = std::env::current_dir()
.map(|p| p.to_string_lossy().to_string())
.unwrap_or_default();
let record = db_writer::ReviewRecord {
let record = review_store::ReviewRecord {
command: cmd,
project_root: &cwd,
commit_hash: commit.as_deref(),
Expand All @@ -730,20 +718,8 @@ async fn execute_chunked_review(
tokens,
issues: &filtered_response.issues,
};
if db_writer::save_review_to_db(&record).is_none() {
debug!("Failed to save review to cora.db");
}

// Auto-resolve findings that no longer appear in this review.
let fps: Vec<String> = filtered_response
.issues
.iter()
.map(db_writer::compute_fingerprint_pub)
.collect();
let resolved = db_writer::resolve_stale_findings(&cwd, &fps);
if resolved > 0 {
debug!(resolved, "auto-resolved stale findings");
}
// Best-effort: a history-write failure never fails the run.
review_store::persist_review_best_effort(&record);
}
let exit_code = compute_exit_code(
gate_result.as_ref().map(|g| g.status),
Expand Down
20 changes: 4 additions & 16 deletions src/commands/scan.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use colored::Colorize;
use tracing::debug;

use crate::config::schema::Config;
use crate::engine::db_writer;
use crate::engine::review_store;
use crate::engine::scanner::{batch_files, format_batch_for_prompt, walk_project};
use crate::engine::types::TokenUsage;
use crate::formatters::{OutputFormat, formatter_for};
Expand Down Expand Up @@ -305,7 +305,7 @@ pub async fn execute_scan(
let cwd = std::env::current_dir()
.map(|p| p.to_string_lossy().to_string())
.unwrap_or_default();
let record = db_writer::ReviewRecord {
let record = review_store::ReviewRecord {
command: "scan",
project_root: &cwd,
commit_hash: commit.as_deref(),
Expand All @@ -318,20 +318,8 @@ pub async fn execute_scan(
tokens: response.tokens_used.as_ref(),
issues: &response.issues,
};
if db_writer::save_review_to_db(&record).is_none() {
debug!("Failed to save scan to cora.db");
}

// Auto-resolve findings that no longer appear in this scan.
let fps: Vec<String> = response
.issues
.iter()
.map(db_writer::compute_fingerprint_pub)
.collect();
let resolved = db_writer::resolve_stale_findings(&cwd, &fps);
if resolved > 0 {
debug!(resolved, "auto-resolved stale findings");
}
// Best-effort: a history-write failure never fails the run.
review_store::persist_review_best_effort(&record);
}

if response.should_block && config.hook.mode == "block" {
Expand Down
Loading
Loading