From 4d8392ac912fd9178f2198448e8fb527585d93a7 Mon Sep 17 00:00:00 2001 From: fagemx Date: Fri, 27 Mar 2026 18:11:31 +0800 Subject: [PATCH 1/3] refactor(edda-ledger): extract domain types to domain.rs (GH-378) Move 9 domain-shaped types (PatternType, DetectedPattern, VillageStats, etc.) from sqlite_store/types.rs to a new domain.rs module. Re-export from lib.rs and update edda-serve to use the new import paths. Phase 1 of 3: no behavioral change, only type relocation. Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/edda-ledger/src/domain.rs | 115 +++++++++++++++++++ crates/edda-ledger/src/lib.rs | 9 +- crates/edda-ledger/src/sqlite_store/types.rs | 114 +----------------- crates/edda-serve/src/api/snapshots.rs | 6 +- 4 files changed, 131 insertions(+), 113 deletions(-) create mode 100644 crates/edda-ledger/src/domain.rs diff --git a/crates/edda-ledger/src/domain.rs b/crates/edda-ledger/src/domain.rs new file mode 100644 index 0000000..8c59c68 --- /dev/null +++ b/crates/edda-ledger/src/domain.rs @@ -0,0 +1,115 @@ +//! Domain types — public API contracts for edda-ledger consumers. +//! +//! These types represent domain concepts that are independent of the storage +//! backend. They are the stable contract for downstream crates. +//! +//! Storage-internal types (e.g. `DecisionRow`) remain in `sqlite_store/types.rs` +//! and are not exposed outside edda-ledger. + +/// The type of recurring pattern detected. +#[derive(Debug, Clone, serde::Serialize)] +#[serde(rename_all = "snake_case")] +pub enum PatternType { + RecurringDecision, + ChiefRepeatedAction, + RollbackTrend, +} + +/// A single detected pattern in a village's decision history. +#[derive(Debug, Clone, serde::Serialize)] +pub struct DetectedPattern { + pub pattern_type: PatternType, + pub key: String, + pub domain: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub authority: Option, + pub occurrences: usize, + pub first_seen: String, + pub last_seen: String, + pub dates: Vec, + pub description: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub trending_up: Option, +} + +/// Result of pattern detection for a village. +#[derive(Debug, Clone, serde::Serialize)] +pub struct PatternDetectionResult { + pub village_id: String, + pub lookback_days: u32, + pub after: String, + pub total_patterns: usize, + pub patterns: Vec, +} + +/// Statistics for a village's decisions. +#[derive(Debug, Clone, serde::Serialize)] +pub struct VillageStats { + pub village_id: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub period: Option, + pub total_decisions: usize, + pub decisions_per_day: f64, + pub by_status: std::collections::HashMap, + pub by_authority: std::collections::HashMap, + pub top_domains: Vec, + pub rollback_rate: f64, + pub trend: Vec, +} + +/// Time period for village stats. +#[derive(Debug, Clone, serde::Serialize)] +pub struct VillageStatsPeriod { + #[serde(skip_serializing_if = "Option::is_none")] + pub after: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub before: Option, +} + +/// Domain with decision count. +#[derive(Debug, Clone, serde::Serialize)] +pub struct DomainCount { + pub domain: String, + pub count: usize, +} + +/// Daily decision count. +#[derive(Debug, Clone, serde::Serialize)] +pub struct DayCount { + pub date: String, + pub count: usize, +} + +/// Aggregated outcome metrics for a decision. +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct OutcomeMetrics { + pub decision_event_id: String, + pub decision_key: String, + pub decision_value: String, + pub decision_ts: String, + pub total_executions: u64, + pub success_count: u64, + pub failed_count: u64, + pub cancelled_count: u64, + pub success_rate: f64, + pub total_cost_usd: f64, + pub total_tokens_in: u64, + pub total_tokens_out: u64, + pub avg_latency_ms: f64, + pub first_execution_ts: Option, + pub last_execution_ts: Option, +} + +/// An execution event linked to a decision. +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct ExecutionLinked { + pub event_id: String, + pub ts: String, + pub status: String, + pub runtime: Option, + pub model: Option, + pub cost_usd: Option, + pub token_in: Option, + pub token_out: Option, + pub latency_ms: Option, +} diff --git a/crates/edda-ledger/src/lib.rs b/crates/edda-ledger/src/lib.rs index 088299a..cdecf22 100644 --- a/crates/edda-ledger/src/lib.rs +++ b/crates/edda-ledger/src/lib.rs @@ -1,6 +1,7 @@ pub mod blob_meta; pub mod blob_store; pub mod device_token; +pub mod domain; pub mod ledger; pub mod lock; pub mod paths; @@ -15,12 +16,16 @@ pub use blob_store::{ blob_put_classified, blob_put_if_large, blob_remove, blob_size, BlobInfo, SNAPSHOT_BLOB_THRESHOLD, }; +pub use domain::{ + DayCount, DetectedPattern, DomainCount, ExecutionLinked, OutcomeMetrics, + PatternDetectionResult, PatternType, VillageStats, VillageStatsPeriod, +}; pub use ledger::Ledger; pub use lock::WorkspaceLock; pub use paths::EddaPaths; pub use sqlite_store::{ - BundleRow, ChainEntry, DecideSnapshotRow, DecisionRow, DepRow, DetectedPattern, DeviceTokenRow, - ImportParams, PatternDetectionResult, PatternType, SuggestionRow, TaskBriefRow, + BundleRow, ChainEntry, DecideSnapshotRow, DecisionRow, DepRow, DeviceTokenRow, ImportParams, + SuggestionRow, TaskBriefRow, }; pub use tombstone::{append_tombstone, list_tombstones, make_tombstone, DeleteReason, Tombstone}; pub use view::DecisionView; diff --git a/crates/edda-ledger/src/sqlite_store/types.rs b/crates/edda-ledger/src/sqlite_store/types.rs index 7b4742b..3c959c5 100644 --- a/crates/edda-ledger/src/sqlite_store/types.rs +++ b/crates/edda-ledger/src/sqlite_store/types.rs @@ -1,5 +1,11 @@ //! Row structs, enums, and parameter types for the SQLite store. +// Re-export domain types so internal sqlite_store code can use them unchanged. +pub use crate::domain::{ + DayCount, DetectedPattern, DomainCount, ExecutionLinked, OutcomeMetrics, + PatternDetectionResult, PatternType, VillageStats, VillageStatsPeriod, +}; + /// A row from the `decisions` table. #[derive(Debug, Clone)] pub struct DecisionRow { @@ -69,80 +75,6 @@ pub struct DepRow { pub created_at: String, } -/// Statistics for a village's decisions. -#[derive(Debug, Clone, serde::Serialize)] -pub struct VillageStats { - pub village_id: String, - #[serde(skip_serializing_if = "Option::is_none")] - pub period: Option, - pub total_decisions: usize, - pub decisions_per_day: f64, - pub by_status: std::collections::HashMap, - pub by_authority: std::collections::HashMap, - pub top_domains: Vec, - pub rollback_rate: f64, - pub trend: Vec, -} - -/// Time period for village stats. -#[derive(Debug, Clone, serde::Serialize)] -pub struct VillageStatsPeriod { - #[serde(skip_serializing_if = "Option::is_none")] - pub after: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub before: Option, -} - -/// The type of recurring pattern detected. -#[derive(Debug, Clone, serde::Serialize)] -#[serde(rename_all = "snake_case")] -pub enum PatternType { - RecurringDecision, - ChiefRepeatedAction, - RollbackTrend, -} - -/// A single detected pattern in a village's decision history. -#[derive(Debug, Clone, serde::Serialize)] -pub struct DetectedPattern { - pub pattern_type: PatternType, - pub key: String, - pub domain: String, - #[serde(skip_serializing_if = "Option::is_none")] - pub authority: Option, - pub occurrences: usize, - pub first_seen: String, - pub last_seen: String, - pub dates: Vec, - pub description: String, - #[serde(skip_serializing_if = "Option::is_none")] - pub trending_up: Option, -} - -/// Result of pattern detection for a village. -#[derive(Debug, Clone, serde::Serialize)] -pub struct PatternDetectionResult { - pub village_id: String, - pub lookback_days: u32, - pub after: String, - pub total_patterns: usize, - pub patterns: Vec, -} - -/// Domain with decision count. -#[derive(Debug, Clone, serde::Serialize)] -pub struct DomainCount { - pub domain: String, - pub count: usize, -} - -/// Daily decision count. -#[derive(Debug, Clone, serde::Serialize)] -pub struct DayCount { - pub date: String, - pub count: usize, -} - /// Parameters for inserting an imported decision from another project. pub struct ImportParams<'a> { pub event: &'a edda_core::types::Event, @@ -215,37 +147,3 @@ pub struct SuggestionRow { pub created_at: String, pub reviewed_at: Option, } - -/// Aggregated outcome metrics for a decision. -#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] -pub struct OutcomeMetrics { - pub decision_event_id: String, - pub decision_key: String, - pub decision_value: String, - pub decision_ts: String, - pub total_executions: u64, - pub success_count: u64, - pub failed_count: u64, - pub cancelled_count: u64, - pub success_rate: f64, - pub total_cost_usd: f64, - pub total_tokens_in: u64, - pub total_tokens_out: u64, - pub avg_latency_ms: f64, - pub first_execution_ts: Option, - pub last_execution_ts: Option, -} - -/// An execution event linked to a decision. -#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] -pub struct ExecutionLinked { - pub event_id: String, - pub ts: String, - pub status: String, - pub runtime: Option, - pub model: Option, - pub cost_usd: Option, - pub token_in: Option, - pub token_out: Option, - pub latency_ms: Option, -} diff --git a/crates/edda-serve/src/api/snapshots.rs b/crates/edda-serve/src/api/snapshots.rs index b1dbbe5..7cdbfba 100644 --- a/crates/edda-serve/src/api/snapshots.rs +++ b/crates/edda-serve/src/api/snapshots.rs @@ -219,7 +219,7 @@ async fn get_village_stats( State(state): State>, AxumPath(village_id): AxumPath, Query(params): Query, -) -> Result, AppError> { +) -> Result, AppError> { if let Some(ref after) = params.after { crate::helpers::validate_iso8601(after).map_err(AppError::Validation)?; } @@ -252,7 +252,7 @@ struct PatternsQuery { async fn get_patterns( State(state): State>, Query(params): Query, -) -> Result, AppError> { +) -> Result, AppError> { let village_id = params .village_id .as_deref() @@ -272,7 +272,7 @@ async fn get_patterns( let patterns = ledger.detect_village_patterns(village_id, &after_str, min_occurrences)?; let total = patterns.len(); - Ok(Json(edda_ledger::sqlite_store::PatternDetectionResult { + Ok(Json(edda_ledger::PatternDetectionResult { village_id: village_id.to_string(), lookback_days, after: after_str, From 1dc09e5c98722d0840eb28ab1b75431ff3638552 Mon Sep 17 00:00:00 2001 From: fagemx Date: Fri, 27 Mar 2026 18:20:51 +0800 Subject: [PATCH 2/3] refactor(edda-ledger): migrate DecisionRow to DecisionView in Ledger API (GH-378) Change 10 Ledger methods to return DecisionView instead of DecisionRow: - active_decisions, active_decisions_limited, decision_timeline, domain_timeline, find_active_decision, shared_decisions, get_decision_by_event_id, active_dependents_of, transitive_dependents_of, causal_chain Add ChainEntryView domain type for causal chain results. Update edda-ask to derive is_active from status field instead of accessing the raw is_active boolean. Update edda-serve chain endpoint similarly. Remove redundant to_view() call in edda-pack. Internal sync.rs bypasses the Ledger facade to access raw rows for source_project_id checking. Phase 2 of 3. Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/edda-ask/src/lib.rs | 15 ++- crates/edda-ledger/src/domain.rs | 11 +++ crates/edda-ledger/src/ledger.rs | 138 ++++++++++++++++++---------- crates/edda-ledger/src/lib.rs | 2 +- crates/edda-ledger/src/sync.rs | 15 ++- crates/edda-pack/src/lib.rs | 5 +- crates/edda-serve/src/api/events.rs | 8 +- 7 files changed, 123 insertions(+), 71 deletions(-) diff --git a/crates/edda-ask/src/lib.rs b/crates/edda-ask/src/lib.rs index 0f7d7ae..e8deca3 100644 --- a/crates/edda-ask/src/lib.rs +++ b/crates/edda-ask/src/lib.rs @@ -1,5 +1,5 @@ use edda_core::Event; -use edda_ledger::sqlite_store::DecisionRow; +use edda_ledger::DecisionView; use edda_ledger::Ledger; use serde::Serialize; @@ -7,7 +7,7 @@ const SEMANTIC_CANDIDATE_LIMIT: usize = 500; #[derive(Debug)] struct ScoredDecision { - row: DecisionRow, + row: DecisionView, score: f64, } @@ -603,7 +603,7 @@ fn semantic_decision_search( fn rank_decisions_by_similarity( query: &str, - candidates: Vec, + candidates: Vec, branch: Option<&str>, ) -> Vec { let query_tokens = tokenize_for_similarity(query); @@ -611,7 +611,7 @@ fn rank_decisions_by_similarity( return vec![]; } - let mut tokenized_docs: Vec<(DecisionRow, Vec)> = Vec::new(); + let mut tokenized_docs: Vec<(DecisionView, Vec)> = Vec::new(); for row in candidates { if branch.is_some_and(|b| row.branch != b) { continue; @@ -810,8 +810,7 @@ pub fn format_human(result: &AskResult) -> String { // ── Internal helpers ───────────────────────────────────────────────── -fn to_decision_hit(row: &DecisionRow) -> DecisionHit { - let tags: Vec = serde_json::from_str(&row.tags).unwrap_or_default(); +fn to_decision_hit(row: &DecisionView) -> DecisionHit { DecisionHit { event_id: row.event_id.clone(), key: row.key.clone(), @@ -820,8 +819,8 @@ fn to_decision_hit(row: &DecisionRow) -> DecisionHit { domain: row.domain.clone(), branch: row.branch.clone(), ts: row.ts.clone().unwrap_or_default(), - is_active: row.is_active, - tags, + is_active: matches!(row.status.as_str(), "active" | "experimental"), + tags: row.tags.clone(), village_id: row.village_id.clone(), } } diff --git a/crates/edda-ledger/src/domain.rs b/crates/edda-ledger/src/domain.rs index 8c59c68..4f294cb 100644 --- a/crates/edda-ledger/src/domain.rs +++ b/crates/edda-ledger/src/domain.rs @@ -100,6 +100,17 @@ pub struct OutcomeMetrics { pub last_execution_ts: Option, } +/// An entry in a causal chain traversal result (domain view). +/// +/// Unlike the internal `ChainEntry` which embeds `DecisionRow`, this type +/// uses `DecisionView` to hide storage details from consumers. +#[derive(Debug, Clone)] +pub struct ChainEntryView { + pub decision: crate::view::DecisionView, + pub relation: String, + pub depth: usize, +} + /// An execution event linked to a decision. #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct ExecutionLinked { diff --git a/crates/edda-ledger/src/ledger.rs b/crates/edda-ledger/src/ledger.rs index b2d33f5..f636637 100644 --- a/crates/edda-ledger/src/ledger.rs +++ b/crates/edda-ledger/src/ledger.rs @@ -1,5 +1,5 @@ use crate::paths::EddaPaths; -use crate::sqlite_store::{BundleRow, DecisionRow, SqliteStore}; +use crate::sqlite_store::{BundleRow, SqliteStore}; use crate::view::{self, DecisionView}; use anyhow::Context; use edda_core::Event; @@ -8,7 +8,7 @@ use std::path::Path; /// The append-only event ledger (SQLite backend). pub struct Ledger { pub paths: EddaPaths, - sqlite: SqliteStore, + pub(crate) sqlite: SqliteStore, } impl Ledger { @@ -209,10 +209,12 @@ impl Ledger { key_pattern: Option<&str>, after: Option<&str>, before: Option<&str>, - ) -> anyhow::Result> { - self.sqlite + ) -> anyhow::Result> { + let rows = self + .sqlite .active_decisions(domain, key_pattern, after, before, None) - .with_context(|| format!("Ledger::active_decisions(domain={domain:?})")) + .with_context(|| format!("Ledger::active_decisions(domain={domain:?})"))?; + Ok(rows.iter().map(view::to_view).collect()) } /// Query active decisions with limit for hot path optimization. @@ -223,10 +225,12 @@ impl Ledger { after: Option<&str>, before: Option<&str>, limit: usize, - ) -> anyhow::Result> { - self.sqlite + ) -> anyhow::Result> { + let rows = self + .sqlite .active_decisions(domain, key_pattern, after, before, Some(limit)) - .with_context(|| format!("Ledger::active_decisions_limited(domain={domain:?})")) + .with_context(|| format!("Ledger::active_decisions_limited(domain={domain:?})"))?; + Ok(rows.iter().map(view::to_view).collect()) } /// All decisions for a key (active + superseded), ordered by time. @@ -236,10 +240,12 @@ impl Ledger { key: &str, after: Option<&str>, before: Option<&str>, - ) -> anyhow::Result> { - self.sqlite + ) -> anyhow::Result> { + let rows = self + .sqlite .decision_timeline(key, after, before) - .with_context(|| format!("Ledger::decision_timeline(key={key})")) + .with_context(|| format!("Ledger::decision_timeline(key={key})"))?; + Ok(rows.iter().map(view::to_view).collect()) } /// All decisions for a domain (active + superseded), ordered by time. @@ -249,10 +255,12 @@ impl Ledger { domain: &str, after: Option<&str>, before: Option<&str>, - ) -> anyhow::Result> { - self.sqlite + ) -> anyhow::Result> { + let rows = self + .sqlite .domain_timeline(domain, after, before) - .with_context(|| format!("Ledger::domain_timeline(domain={domain})")) + .with_context(|| format!("Ledger::domain_timeline(domain={domain})"))?; + Ok(rows.iter().map(view::to_view).collect()) } /// Distinct domain values from active decisions. @@ -266,7 +274,7 @@ impl Ledger { village_id: &str, after: Option<&str>, before: Option<&str>, - ) -> anyhow::Result { + ) -> anyhow::Result { self.sqlite .village_stats(village_id, after, before) .with_context(|| format!("Ledger::village_stats({village_id})")) @@ -278,7 +286,7 @@ impl Ledger { village_id: &str, after: &str, min_occurrences: usize, - ) -> anyhow::Result> { + ) -> anyhow::Result> { self.sqlite .detect_village_patterns(village_id, after, min_occurrences) .with_context(|| format!("Ledger::detect_village_patterns({village_id})")) @@ -289,10 +297,12 @@ impl Ledger { &self, branch: &str, key: &str, - ) -> anyhow::Result> { - self.sqlite + ) -> anyhow::Result> { + let row = self + .sqlite .find_active_decision(branch, key) - .with_context(|| format!("Ledger::find_active_decision(branch={branch}, key={key})")) + .with_context(|| format!("Ledger::find_active_decision(branch={branch}, key={key})"))?; + Ok(row.as_ref().map(view::to_view)) } /// Return active decisions that have non-empty `affected_paths`. @@ -356,10 +366,12 @@ impl Ledger { // ── Cross-Project Sync ──────────────────────────────────────────── /// Query active decisions with shared or global scope. - pub fn shared_decisions(&self) -> anyhow::Result> { - self.sqlite + pub fn shared_decisions(&self) -> anyhow::Result> { + let rows = self + .sqlite .shared_decisions() - .context("Ledger::shared_decisions") + .context("Ledger::shared_decisions")?; + Ok(rows.iter().map(view::to_view).collect()) } /// Check if a decision has already been imported from a source project. @@ -376,7 +388,7 @@ impl Ledger { /// Insert an imported decision from another project. pub fn insert_imported_decision( &self, - params: crate::sqlite_store::ImportParams<'_>, + params: crate::ImportParams<'_>, ) -> anyhow::Result<()> { self.sqlite .insert_imported_decision(params) @@ -416,10 +428,15 @@ impl Ledger { pub fn active_dependents_of( &self, key: &str, - ) -> anyhow::Result> { - self.sqlite + ) -> anyhow::Result> { + let rows = self + .sqlite .active_dependents_of(key) - .with_context(|| format!("Ledger::active_dependents_of({key})")) + .with_context(|| format!("Ledger::active_dependents_of({key})"))?; + Ok(rows + .into_iter() + .map(|(dep, row)| (dep, view::to_view(&row))) + .collect()) } // ── Decision Outcomes ───────────────────────────────────────────── @@ -428,7 +445,7 @@ impl Ledger { pub fn decision_outcomes( &self, decision_event_id: &str, - ) -> anyhow::Result> { + ) -> anyhow::Result> { self.sqlite .decision_outcomes(decision_event_id) .with_context(|| format!("Ledger::decision_outcomes({decision_event_id})")) @@ -438,31 +455,41 @@ impl Ledger { pub fn executions_for_decision( &self, decision_event_id: &str, - ) -> anyhow::Result> { + ) -> anyhow::Result> { self.sqlite .executions_for_decision(decision_event_id) .with_context(|| format!("Ledger::executions_for_decision({decision_event_id})")) } /// Transitive dependents of `key` via BFS, up to `max_depth` hops. - /// Returns `(DepRow, DecisionRow, depth)` — only active decisions, deduplicated. + /// Returns `(DepRow, DecisionView, depth)` — only active decisions, deduplicated. pub fn transitive_dependents_of( &self, key: &str, max_depth: usize, - ) -> anyhow::Result> { - self.sqlite + ) -> anyhow::Result> { + let rows = self + .sqlite .transitive_dependents_of(key, max_depth) - .with_context(|| format!("Ledger::transitive_dependents_of({key})")) + .with_context(|| format!("Ledger::transitive_dependents_of({key})"))?; + Ok(rows + .into_iter() + .map(|(dep, row, depth)| (dep, view::to_view(&row), depth)) + .collect()) } // ── Causal Chain ───────────────────────────────────────────────── /// Look up a single decision by event_id. - pub fn get_decision_by_event_id(&self, event_id: &str) -> anyhow::Result> { - self.sqlite + pub fn get_decision_by_event_id( + &self, + event_id: &str, + ) -> anyhow::Result> { + let row = self + .sqlite .get_decision_by_event_id(event_id) - .with_context(|| format!("Ledger::get_decision_by_event_id({event_id})")) + .with_context(|| format!("Ledger::get_decision_by_event_id({event_id})"))?; + Ok(row.as_ref().map(view::to_view)) } /// Traverse the causal chain from a root decision via unified BFS. @@ -470,10 +497,23 @@ impl Ledger { &self, event_id: &str, max_depth: usize, - ) -> anyhow::Result)>> { - self.sqlite + ) -> anyhow::Result)>> { + let result = self + .sqlite .causal_chain(event_id, max_depth) - .with_context(|| format!("Ledger::causal_chain({event_id})")) + .with_context(|| format!("Ledger::causal_chain({event_id})"))?; + Ok(result.map(|(root, entries)| { + let root_view = view::to_view(&root); + let entry_views = entries + .into_iter() + .map(|e| crate::domain::ChainEntryView { + decision: view::to_view(&e.decision), + relation: e.relation, + depth: e.depth, + }) + .collect(); + (root_view, entry_views) + })) } // ── Task Briefs ────────────────────────────────────────────────── @@ -482,7 +522,7 @@ impl Ledger { pub fn get_task_brief( &self, task_id: &str, - ) -> anyhow::Result> { + ) -> anyhow::Result> { self.sqlite .get_task_brief(task_id) .with_context(|| format!("Ledger::get_task_brief({task_id})")) @@ -493,7 +533,7 @@ impl Ledger { &self, status: Option<&str>, intent: Option<&str>, - ) -> anyhow::Result> { + ) -> anyhow::Result> { self.sqlite .list_task_briefs(status, intent) .context("Ledger::list_task_briefs") @@ -520,7 +560,7 @@ impl Ledger { /// Insert a new device token row. pub fn insert_device_token( &self, - row: &crate::sqlite_store::DeviceTokenRow, + row: &crate::DeviceTokenRow, ) -> anyhow::Result<()> { self.sqlite .insert_device_token(row) @@ -531,14 +571,14 @@ impl Ledger { pub fn validate_device_token( &self, token_hash: &str, - ) -> anyhow::Result> { + ) -> anyhow::Result> { self.sqlite .validate_device_token(token_hash) .context("Ledger::validate_device_token") } /// List all device tokens (active and revoked). - pub fn list_device_tokens(&self) -> anyhow::Result> { + pub fn list_device_tokens(&self) -> anyhow::Result> { self.sqlite .list_device_tokens() .context("Ledger::list_device_tokens") @@ -567,7 +607,7 @@ impl Ledger { /// Insert a row into the `decide_snapshots` materialized view. pub fn insert_snapshot( &self, - row: &crate::sqlite_store::DecideSnapshotRow, + row: &crate::DecideSnapshotRow, ) -> anyhow::Result<()> { self.sqlite .insert_snapshot(row) @@ -580,7 +620,7 @@ impl Ledger { village_id: Option<&str>, engine_version: Option<&str>, limit: usize, - ) -> anyhow::Result> { + ) -> anyhow::Result> { self.sqlite .query_snapshots(village_id, engine_version, limit) .context("Ledger::query_snapshots") @@ -590,7 +630,7 @@ impl Ledger { pub fn snapshots_by_context_hash( &self, context_hash: &str, - ) -> anyhow::Result> { + ) -> anyhow::Result> { self.sqlite .snapshots_by_context_hash(context_hash) .with_context(|| format!("Ledger::snapshots_by_context_hash({context_hash})")) @@ -601,7 +641,7 @@ impl Ledger { /// Insert a new suggestion row. pub fn insert_suggestion( &self, - row: &crate::sqlite_store::SuggestionRow, + row: &crate::SuggestionRow, ) -> anyhow::Result<()> { self.sqlite .insert_suggestion(row) @@ -612,7 +652,7 @@ impl Ledger { pub fn list_suggestions_by_status( &self, status: &str, - ) -> anyhow::Result> { + ) -> anyhow::Result> { self.sqlite .list_suggestions_by_status(status) .with_context(|| format!("Ledger::list_suggestions_by_status({status})")) @@ -622,7 +662,7 @@ impl Ledger { pub fn get_suggestion( &self, id: &str, - ) -> anyhow::Result> { + ) -> anyhow::Result> { self.sqlite .get_suggestion(id) .with_context(|| format!("Ledger::get_suggestion({id})")) diff --git a/crates/edda-ledger/src/lib.rs b/crates/edda-ledger/src/lib.rs index cdecf22..55a7bbf 100644 --- a/crates/edda-ledger/src/lib.rs +++ b/crates/edda-ledger/src/lib.rs @@ -17,7 +17,7 @@ pub use blob_store::{ SNAPSHOT_BLOB_THRESHOLD, }; pub use domain::{ - DayCount, DetectedPattern, DomainCount, ExecutionLinked, OutcomeMetrics, + ChainEntryView, DayCount, DetectedPattern, DomainCount, ExecutionLinked, OutcomeMetrics, PatternDetectionResult, PatternType, VillageStats, VillageStatsPeriod, }; pub use ledger::Ledger; diff --git a/crates/edda-ledger/src/sync.rs b/crates/edda-ledger/src/sync.rs index 3c12eaf..f0c3a62 100644 --- a/crates/edda-ledger/src/sync.rs +++ b/crates/edda-ledger/src/sync.rs @@ -92,7 +92,9 @@ pub fn sync_from_sources( } }; - let shared = match source_ledger.shared_decisions() { + // Use internal SqliteStore to get raw DecisionRow (sync needs scope, + // source_project_id fields not exposed via DecisionView). + let shared = match source_ledger.sqlite.shared_decisions() { Ok(d) => d, Err(e) => { result.errors.push(SourceError { @@ -110,8 +112,8 @@ pub fn sync_from_sources( continue; } - // Check for local conflict - let local = target.find_active_decision(&branch, &decision.key)?; + // Check for local conflict (use raw row to access source_project_id) + let local = target.sqlite.find_active_decision(&branch, &decision.key)?; let is_conflict = local .as_ref() .map(|l| l.value != decision.value && l.source_project_id.is_none()) @@ -319,8 +321,11 @@ mod tests { assert_eq!(result.imported[0].key, "api.version"); assert_eq!(result.imported[0].value, "v3"); - // Verify it was written to the ledger - let decisions = tgt_ledger.active_decisions(None, None, None, None).unwrap(); + // Verify it was written to the ledger (use raw rows to check source_project_id) + let decisions = tgt_ledger + .sqlite + .active_decisions(None, None, None, None, None) + .unwrap(); assert!(decisions.iter().any(|d| d.key == "api.version" && d.value == "v3" && d.source_project_id.as_deref() == Some("source_proj"))); diff --git a/crates/edda-pack/src/lib.rs b/crates/edda-pack/src/lib.rs index 20d889b..b069aeb 100644 --- a/crates/edda-pack/src/lib.rs +++ b/crates/edda-pack/src/lib.rs @@ -434,10 +434,7 @@ pub fn build_decision_pack(repo_root: &Path, branch: &str, max_items: usize) -> let views: Vec = match edda_ledger::Ledger::open(repo_root) { Ok(ledger) => ledger .active_decisions_limited(None, None, None, None, max_items) - .unwrap_or_default() - .iter() - .map(edda_ledger::view::to_view) - .collect(), + .unwrap_or_default(), Err(_) => Vec::new(), }; diff --git a/crates/edda-serve/src/api/events.rs b/crates/edda-serve/src/api/events.rs index 6fb1d22..a84b836 100644 --- a/crates/edda-serve/src/api/events.rs +++ b/crates/edda-serve/src/api/events.rs @@ -330,27 +330,27 @@ async fn get_decision_chain( .ok_or_else(|| AppError::NotFound(format!("decision not found: {}", event_id)))?; let root_node = ChainNodeResponse { - event_id: root.event_id, + event_id: root.event_id.clone(), key: root.key, value: root.value, reason: root.reason, relation: None, depth: None, ts: root.ts.unwrap_or_default(), - is_active: root.is_active, + is_active: matches!(root.status.as_str(), "active" | "experimental"), }; let chain_nodes: Vec = chain .into_iter() .map(|entry| ChainNodeResponse { - event_id: entry.decision.event_id, + event_id: entry.decision.event_id.clone(), key: entry.decision.key, value: entry.decision.value, reason: entry.decision.reason, relation: Some(entry.relation), depth: Some(entry.depth), ts: entry.decision.ts.unwrap_or_default(), - is_active: entry.decision.is_active, + is_active: matches!(entry.decision.status.as_str(), "active" | "experimental"), }) .collect(); From c021c0091ab562d3e8cbaacd6eefade42b9f9635 Mon Sep 17 00:00:00 2001 From: fagemx Date: Fri, 27 Mar 2026 18:33:43 +0800 Subject: [PATCH 3/3] refactor(edda-ledger): seal sqlite_store module and rename DepRow (GH-378) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move remaining public types (BundleRow, DeviceTokenRow, DecideSnapshotRow, SuggestionRow, TaskBriefRow, ImportParams) from sqlite_store/types.rs to domain.rs. Rename DepRow to DependencyEdge with backwards-compatible alias. Change sqlite_store module visibility to pub(crate) — no downstream crate can reach sqlite_store::* directly anymore. All types are re-exported from lib.rs via the domain module. Phase 3 of 3: completes the domain abstraction boundary. Co-Authored-By: Claude Opus 4.6 (1M context) --- crates/edda-ledger/src/domain.rs | 102 ++++++++++++++++ crates/edda-ledger/src/ledger.rs | 45 ++----- crates/edda-ledger/src/lib.rs | 12 +- crates/edda-ledger/src/sqlite_store/events.rs | 1 + crates/edda-ledger/src/sqlite_store/types.rs | 115 ++---------------- crates/edda-serve/src/lib.rs | 4 +- 6 files changed, 133 insertions(+), 146 deletions(-) diff --git a/crates/edda-ledger/src/domain.rs b/crates/edda-ledger/src/domain.rs index 4f294cb..ced530e 100644 --- a/crates/edda-ledger/src/domain.rs +++ b/crates/edda-ledger/src/domain.rs @@ -124,3 +124,105 @@ pub struct ExecutionLinked { pub token_out: Option, pub latency_ms: Option, } + +// ── Types moved from sqlite_store/types.rs ────────────────────────── + +/// A review bundle row. +#[derive(Debug, Clone)] +pub struct BundleRow { + pub event_id: String, + pub bundle_id: String, + pub status: String, + pub risk_level: String, + pub total_added: i64, + pub total_deleted: i64, + pub files_changed: i64, + pub tests_passed: i64, + pub tests_failed: i64, + pub suggested_action: String, + pub branch: String, + pub created_at: String, +} + +/// A dependency edge between two decision keys. +#[derive(Debug, Clone)] +pub struct DependencyEdge { + pub source_key: String, + pub target_key: String, + pub dep_type: String, + pub created_event: Option, + pub created_at: String, +} + +/// Parameters for inserting an imported decision from another project. +pub struct ImportParams<'a> { + pub event: &'a edda_core::types::Event, + pub key: &'a str, + pub value: &'a str, + pub reason: &'a str, + pub domain: &'a str, + pub scope: &'a str, + pub source_project_id: &'a str, + pub source_event_id: &'a str, + pub is_active: bool, +} + +/// A task brief row. +#[derive(Debug, Clone)] +pub struct TaskBriefRow { + pub task_id: String, + pub intake_event_id: String, + pub title: String, + pub intent: edda_core::types::TaskBriefIntent, + pub source_url: String, + pub status: edda_core::types::TaskBriefStatus, + pub branch: String, + pub iterations: i64, + pub artifacts: String, + pub decisions: String, + pub last_feedback: Option, + pub created_at: String, + pub updated_at: String, +} + +/// A device token row. +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct DeviceTokenRow { + pub token_hash: String, + pub device_name: String, + pub paired_at: String, + pub paired_from_ip: String, + pub revoked_at: Option, + pub pair_event_id: String, + pub revoke_event_id: Option, +} + +/// A decide snapshot row. +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct DecideSnapshotRow { + pub event_id: String, + pub context_hash: String, + pub engine_version: String, + pub schema_version: String, + pub redaction_level: String, + pub village_id: Option, + pub cycle_id: Option, + pub has_blobs: bool, + pub created_at: String, +} + +/// A suggestion row. +#[derive(Debug, Clone)] +pub struct SuggestionRow { + pub id: String, + pub event_type: String, + pub source_layer: String, + pub source_refs: String, + pub summary: String, + pub suggested_because: String, + pub detail: String, + pub tags: String, + pub status: String, + pub created_at: String, + pub reviewed_at: Option, +} diff --git a/crates/edda-ledger/src/ledger.rs b/crates/edda-ledger/src/ledger.rs index f636637..ba53b3d 100644 --- a/crates/edda-ledger/src/ledger.rs +++ b/crates/edda-ledger/src/ledger.rs @@ -386,10 +386,7 @@ impl Ledger { } /// Insert an imported decision from another project. - pub fn insert_imported_decision( - &self, - params: crate::ImportParams<'_>, - ) -> anyhow::Result<()> { + pub fn insert_imported_decision(&self, params: crate::ImportParams<'_>) -> anyhow::Result<()> { self.sqlite .insert_imported_decision(params) .context("Ledger::insert_imported_decision") @@ -411,14 +408,14 @@ impl Ledger { } /// What does `key` depend on? - pub fn deps_of(&self, key: &str) -> anyhow::Result> { + pub fn deps_of(&self, key: &str) -> anyhow::Result> { self.sqlite .deps_of(key) .with_context(|| format!("Ledger::deps_of({key})")) } /// Who depends on `key`? - pub fn dependents_of(&self, key: &str) -> anyhow::Result> { + pub fn dependents_of(&self, key: &str) -> anyhow::Result> { self.sqlite .dependents_of(key) .with_context(|| format!("Ledger::dependents_of({key})")) @@ -428,7 +425,7 @@ impl Ledger { pub fn active_dependents_of( &self, key: &str, - ) -> anyhow::Result> { + ) -> anyhow::Result> { let rows = self .sqlite .active_dependents_of(key) @@ -462,12 +459,12 @@ impl Ledger { } /// Transitive dependents of `key` via BFS, up to `max_depth` hops. - /// Returns `(DepRow, DecisionView, depth)` — only active decisions, deduplicated. + /// Returns `(DependencyEdge, DecisionView, depth)` — only active decisions, deduplicated. pub fn transitive_dependents_of( &self, key: &str, max_depth: usize, - ) -> anyhow::Result> { + ) -> anyhow::Result> { let rows = self .sqlite .transitive_dependents_of(key, max_depth) @@ -481,10 +478,7 @@ impl Ledger { // ── Causal Chain ───────────────────────────────────────────────── /// Look up a single decision by event_id. - pub fn get_decision_by_event_id( - &self, - event_id: &str, - ) -> anyhow::Result> { + pub fn get_decision_by_event_id(&self, event_id: &str) -> anyhow::Result> { let row = self .sqlite .get_decision_by_event_id(event_id) @@ -519,10 +513,7 @@ impl Ledger { // ── Task Briefs ────────────────────────────────────────────────── /// Get a task brief by task_id. - pub fn get_task_brief( - &self, - task_id: &str, - ) -> anyhow::Result> { + pub fn get_task_brief(&self, task_id: &str) -> anyhow::Result> { self.sqlite .get_task_brief(task_id) .with_context(|| format!("Ledger::get_task_brief({task_id})")) @@ -558,10 +549,7 @@ impl Ledger { // ── Device Tokens ─────────────────────────────────────────────── /// Insert a new device token row. - pub fn insert_device_token( - &self, - row: &crate::DeviceTokenRow, - ) -> anyhow::Result<()> { + pub fn insert_device_token(&self, row: &crate::DeviceTokenRow) -> anyhow::Result<()> { self.sqlite .insert_device_token(row) .context("Ledger::insert_device_token") @@ -605,10 +593,7 @@ impl Ledger { // ── Decide Snapshots ──────────────────────────────────────────── /// Insert a row into the `decide_snapshots` materialized view. - pub fn insert_snapshot( - &self, - row: &crate::DecideSnapshotRow, - ) -> anyhow::Result<()> { + pub fn insert_snapshot(&self, row: &crate::DecideSnapshotRow) -> anyhow::Result<()> { self.sqlite .insert_snapshot(row) .context("Ledger::insert_snapshot") @@ -639,10 +624,7 @@ impl Ledger { // ── Suggestions ────────────────────────────────────────────────── /// Insert a new suggestion row. - pub fn insert_suggestion( - &self, - row: &crate::SuggestionRow, - ) -> anyhow::Result<()> { + pub fn insert_suggestion(&self, row: &crate::SuggestionRow) -> anyhow::Result<()> { self.sqlite .insert_suggestion(row) .context("Ledger::insert_suggestion") @@ -659,10 +641,7 @@ impl Ledger { } /// Get a single suggestion by id. - pub fn get_suggestion( - &self, - id: &str, - ) -> anyhow::Result> { + pub fn get_suggestion(&self, id: &str) -> anyhow::Result> { self.sqlite .get_suggestion(id) .with_context(|| format!("Ledger::get_suggestion({id})")) diff --git a/crates/edda-ledger/src/lib.rs b/crates/edda-ledger/src/lib.rs index 55a7bbf..3e50886 100644 --- a/crates/edda-ledger/src/lib.rs +++ b/crates/edda-ledger/src/lib.rs @@ -5,7 +5,7 @@ pub mod domain; pub mod ledger; pub mod lock; pub mod paths; -pub mod sqlite_store; +pub(crate) mod sqlite_store; pub mod sync; pub mod tombstone; pub mod view; @@ -17,15 +17,13 @@ pub use blob_store::{ SNAPSHOT_BLOB_THRESHOLD, }; pub use domain::{ - ChainEntryView, DayCount, DetectedPattern, DomainCount, ExecutionLinked, OutcomeMetrics, - PatternDetectionResult, PatternType, VillageStats, VillageStatsPeriod, + BundleRow, ChainEntryView, DayCount, DecideSnapshotRow, DependencyEdge, DetectedPattern, + DeviceTokenRow, DomainCount, ExecutionLinked, ImportParams, OutcomeMetrics, + PatternDetectionResult, PatternType, SuggestionRow, TaskBriefRow, VillageStats, + VillageStatsPeriod, }; pub use ledger::Ledger; pub use lock::WorkspaceLock; pub use paths::EddaPaths; -pub use sqlite_store::{ - BundleRow, ChainEntry, DecideSnapshotRow, DecisionRow, DepRow, DeviceTokenRow, ImportParams, - SuggestionRow, TaskBriefRow, -}; pub use tombstone::{append_tombstone, list_tombstones, make_tombstone, DeleteReason, Tombstone}; pub use view::DecisionView; diff --git a/crates/edda-ledger/src/sqlite_store/events.rs b/crates/edda-ledger/src/sqlite_store/events.rs index 599676a..fa5353d 100644 --- a/crates/edda-ledger/src/sqlite_store/events.rs +++ b/crates/edda-ledger/src/sqlite_store/events.rs @@ -551,6 +551,7 @@ impl SqliteStore { /// matches the previous event's `hash`. /// /// Returns `Err` describing the first break found. + #[cfg_attr(not(test), allow(dead_code))] pub fn verify_chain(&self) -> anyhow::Result<()> { let events = self.iter_events()?; if events.is_empty() { diff --git a/crates/edda-ledger/src/sqlite_store/types.rs b/crates/edda-ledger/src/sqlite_store/types.rs index 3c959c5..a0e64f5 100644 --- a/crates/edda-ledger/src/sqlite_store/types.rs +++ b/crates/edda-ledger/src/sqlite_store/types.rs @@ -1,12 +1,19 @@ //! Row structs, enums, and parameter types for the SQLite store. +//! +//! Only storage-internal types are defined here. Public domain types +//! live in `crate::domain` and are re-exported for internal use. // Re-export domain types so internal sqlite_store code can use them unchanged. pub use crate::domain::{ - DayCount, DetectedPattern, DomainCount, ExecutionLinked, OutcomeMetrics, - PatternDetectionResult, PatternType, VillageStats, VillageStatsPeriod, + BundleRow, DayCount, DecideSnapshotRow, DependencyEdge, DetectedPattern, DeviceTokenRow, + DomainCount, ExecutionLinked, ImportParams, OutcomeMetrics, PatternType, SuggestionRow, + TaskBriefRow, VillageStats, VillageStatsPeriod, }; -/// A row from the `decisions` table. +/// Backwards-compatible alias: `DepRow` → `DependencyEdge`. +pub type DepRow = DependencyEdge; + +/// A row from the `decisions` table (storage-internal). #[derive(Debug, Clone)] pub struct DecisionRow { pub event_id: String, @@ -40,110 +47,10 @@ pub struct DecisionRow { pub village_id: Option, } -/// An entry in a causal chain traversal result. +/// An entry in a causal chain traversal result (storage-internal). #[derive(Debug, Clone)] pub struct ChainEntry { pub decision: DecisionRow, pub relation: String, pub depth: usize, } - -/// A row from the `review_bundles` table. -#[derive(Debug, Clone)] -pub struct BundleRow { - pub event_id: String, - pub bundle_id: String, - pub status: String, - pub risk_level: String, - pub total_added: i64, - pub total_deleted: i64, - pub files_changed: i64, - pub tests_passed: i64, - pub tests_failed: i64, - pub suggested_action: String, - pub branch: String, - pub created_at: String, -} - -/// A row from the `decision_deps` table. -#[derive(Debug, Clone)] -pub struct DepRow { - pub source_key: String, - pub target_key: String, - pub dep_type: String, - pub created_event: Option, - pub created_at: String, -} - -/// Parameters for inserting an imported decision from another project. -pub struct ImportParams<'a> { - pub event: &'a edda_core::types::Event, - pub key: &'a str, - pub value: &'a str, - pub reason: &'a str, - pub domain: &'a str, - pub scope: &'a str, - pub source_project_id: &'a str, - pub source_event_id: &'a str, - pub is_active: bool, -} - -/// A row from the `task_briefs` table. -#[derive(Debug, Clone)] -pub struct TaskBriefRow { - pub task_id: String, - pub intake_event_id: String, - pub title: String, - pub intent: edda_core::types::TaskBriefIntent, - pub source_url: String, - pub status: edda_core::types::TaskBriefStatus, - pub branch: String, - pub iterations: i64, - pub artifacts: String, - pub decisions: String, - pub last_feedback: Option, - pub created_at: String, - pub updated_at: String, -} - -/// A row from the `device_tokens` table. -#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] -pub struct DeviceTokenRow { - pub token_hash: String, - pub device_name: String, - pub paired_at: String, - pub paired_from_ip: String, - pub revoked_at: Option, - pub pair_event_id: String, - pub revoke_event_id: Option, -} - -/// A row from the `decide_snapshots` table. -#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] -pub struct DecideSnapshotRow { - pub event_id: String, - pub context_hash: String, - pub engine_version: String, - pub schema_version: String, - pub redaction_level: String, - pub village_id: Option, - pub cycle_id: Option, - pub has_blobs: bool, - pub created_at: String, -} - -/// A row from the `suggestions` table. -#[derive(Debug, Clone)] -pub struct SuggestionRow { - pub id: String, - pub event_type: String, - pub source_layer: String, - pub source_refs: String, - pub summary: String, - pub suggested_because: String, - pub detail: String, - pub tags: String, - pub status: String, - pub created_at: String, - pub reviewed_at: Option, -} diff --git a/crates/edda-serve/src/lib.rs b/crates/edda-serve/src/lib.rs index 2730e83..54652a9 100644 --- a/crates/edda-serve/src/lib.rs +++ b/crates/edda-serve/src/lib.rs @@ -3785,7 +3785,7 @@ actors: let pair_evt = seed_dummy_event(&ledger); let revoke_evt = seed_dummy_event(&ledger); ledger - .insert_device_token(&edda_ledger::sqlite_store::DeviceTokenRow { + .insert_device_token(&edda_ledger::DeviceTokenRow { token_hash: token_hash.clone(), device_name: "test-device".to_string(), paired_at: "2026-01-01T00:00:00Z".to_string(), @@ -3816,7 +3816,7 @@ actors: let ledger = Ledger::open(tmp.path()).unwrap(); let pair_evt = seed_dummy_event(&ledger); ledger - .insert_device_token(&edda_ledger::sqlite_store::DeviceTokenRow { + .insert_device_token(&edda_ledger::DeviceTokenRow { token_hash, device_name: "test-device".to_string(), paired_at: "2026-01-01T00:00:00Z".to_string(),