jake/Lumotia
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

agent: area A commit 1 — typed magnotia_storage::Error + boundary

Browse Source 
Commit 1 of 2 for engine-slop residuals Area A.

Adds a typed `magnotia_storage::Error` enum and rewires every error
construction site in the storage crate to use it. The crate boundary
into `magnotia_core::error::MagnotiaError` is handled by a
`From<storage::Error> for MagnotiaError` impl that lives inside the
storage crate (not in core) to avoid a `core -> storage` dependency
cycle. The old `MagnotiaError::StorageError(String)` variant is kept
in this commit as an unused placeholder; commit 2 deletes it.

New types in crates/storage/src/error.rs:
- pub enum Error { DatabaseOpen, Migration, Query, NotFound,
  InvalidReference, Filesystem } with #[derive(thiserror::Error)]
- pub enum OpenOp { Connect, ReadOnlyConnect, ForeignKeysPragma }
- pub enum MigrationStep { SchemaVersionTableCreate, SchemaVersionQuery,
  TxBegin, Apply, RecordVersion, Commit }
- pub enum Entity { Transcript, Task, Profile, ImplementationRule,
  Feedback } — seeded only with entities that have a real
  NotFound/InvalidReference case in the codebase
- pub type Result<T> = std::result::Result<T, Error>
- impl Error { pub fn kind() -> StorageKind, pub fn
  operation_label() -> Cow<'static, str> }
- impl From<Error> for MagnotiaError

New types in crates/core/src/error.rs:
- MagnotiaError::Storage { kind: StorageKind, operation: String,
  detail: String } with #[error("{detail}")] so the boundary doesn't
  double-prefix Display output
- pub enum StorageKind { DatabaseOpen, Migration, Query, NotFound,
  InvalidReference, Filesystem } #[derive(Serialize)]
  #[serde(rename_all = "snake_case")] for future Area E

Storage crate dependency added: thiserror = "1"

Migration scope:
- migrations.rs: 6 sites → Error::Migration with structured step + version
- database.rs: 72 sites broken down as:
  * 3  → Error::DatabaseOpen (init / readonly / pragma)
  * 1  → Error::Filesystem (init's create_dir_all with path context)
  * ~58 → Error::Query with operation labels matching the prior message
        stem in snake_case; transaction sub-steps use dotted labels
        ("complete_subtask_and_check_parent.commit_transaction" etc.)
        so transaction internals are not collapsed
  * 5  → Error::NotFound (transcript / task×2 / implementation_rule×2)
  * 5  → Error::InvalidReference (insert_transcript unknown profile;
        Default profile rename/delete invariants ×3; feedback rating
        value validation)

Survey ambiguities resolved per the locked answers:
- Q1 (schema_version_query): Migration step, not Query
- Q2 (transaction sub-steps): preserved granularity with dotted operation
  labels; no collapsing
- Q3 (Entity cardinality): seeded with the 3 from the survey + 2 more
  discovered during migration (ImplementationRule, Feedback), per
  "add when an actual case needs it"
- Q4 (operation label type): Cow<'static, str>
- Q5 (Serialize): storage::Error is not serializable; flattens only at
  the MagnotiaError boundary
- Q6 (re-exports): pub use error::{Entity, Error, MigrationStep, OpenOp,
  Result} in storage::lib.rs; StorageKind belongs to magnotia_core

Two scope-discovered additions beyond the original 5 variants:
- Error::Filesystem { path, source } variant + matching StorageKind —
  required because init() now returns storage::Result<SqlitePool> and
  the create_dir_all call needs a typed path; doing this via
  From<io::Error> for storage::Error would have lost the path so it's
  explicit
- Entity::ImplementationRule and Entity::Feedback — two NotFound /
  InvalidReference sites the original survey missed in the rule-CRUD
  and feedback-validation areas

Verification:
- cargo fmt --all
- cargo check --workspace --all-targets — clean
- cargo test -p magnotia-storage — 60 passed, 0 failed
- cargo test --workspace — all green, ~330 tests
- rg 'StorageError\(' crates/ src-tauri/src/ — only hit is the variant
  declaration in core that commit 2 will delete
- rg 'Other\(String' crates/storage/src/ — zero
- rg 'format!\("[A-Z].* failed' crates/storage/src/ — zero

Commit 2 will delete MagnotiaError::StorageError(String) once this is
in. Pre-existing working-tree churn in crates/llm/, src/lib/pages/,
src/lib/utils/saveMarkdown.ts and the untracked phase10a dogfood notes
deliberately left unstaged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jake
2026-05-12 22:57:17 +01:00
parent fdab77776c
commit 52565ea8b8
7 changed files with 550 additions and 137 deletions
Download Patch File Download Diff File Expand all files Collapse all files

194
crates/storage/src/error.rs Normal file
View File

@@ -0,0 +1,194 @@
//! Storage-local typed error.
//!
//! Backend code that wants to branch on storage failure modes pattern-matches
//! on [`Error`] directly. The crate boundary into [`magnotia_core::error::MagnotiaError`]
//! flattens this into a [`MagnotiaError::Storage`] variant carrying a serde-friendly
//! `kind`, an `operation` label, and the Display output as `detail` — so the
//! frontend (which still receives stringified errors from Tauri commands today)
//! keeps working, and Area E can later expose the typed `kind` to the FE without
//! touching call sites.
//!
//! The `sqlx::Error` source is deliberately not serialized. Sources stay sources;
//! only the boundary shape is wire-friendly.
use std::borrow::Cow;
use std::path::PathBuf;
use magnotia_core::error::{MagnotiaError, StorageKind};
/// Kinds of database-open operation that can fail before the pool is ready.
#[derive(Debug, Clone, Copy)]
pub enum OpenOp {
Connect,
ReadOnlyConnect,
ForeignKeysPragma,
}
impl OpenOp {
fn label(self) -> &'static str {
match self {
OpenOp::Connect => "connect",
OpenOp::ReadOnlyConnect => "read_only_connect",
OpenOp::ForeignKeysPragma => "foreign_keys_pragma",
}
}
}
impl std::fmt::Display for OpenOp {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(self.label())
}
}
/// Per-step phase inside the migration framework.
#[derive(Debug, Clone, Copy)]
pub enum MigrationStep {
SchemaVersionTableCreate,
SchemaVersionQuery,
TxBegin,
Apply,
RecordVersion,
Commit,
}
impl MigrationStep {
fn label(self) -> &'static str {
match self {
MigrationStep::SchemaVersionTableCreate => "schema_version_table_create",
MigrationStep::SchemaVersionQuery => "schema_version_query",
MigrationStep::TxBegin => "tx_begin",
MigrationStep::Apply => "apply",
MigrationStep::RecordVersion => "record_version",
MigrationStep::Commit => "commit",
}
}
}
impl std::fmt::Display for MigrationStep {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(self.label())
}
}
/// Entities the storage layer can fail to find or validate.
///
/// Seeded with only the three real entities (Transcript, Task, Profile) — add
/// more here only when a typed `NotFound` / `InvalidReference` case actually
/// requires them.
#[derive(Debug, Clone, Copy)]
pub enum Entity {
Transcript,
Task,
Profile,
ImplementationRule,
Feedback,
}
impl Entity {
fn label(self) -> &'static str {
match self {
Entity::Transcript => "transcript",
Entity::Task => "task",
Entity::Profile => "profile",
Entity::ImplementationRule => "implementation_rule",
Entity::Feedback => "feedback",
}
}
}
impl std::fmt::Display for Entity {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(self.label())
}
}
/// Storage-crate-local typed error.
#[derive(Debug, thiserror::Error)]
pub enum Error {
#[error("database open failed during {operation}: {source}")]
DatabaseOpen {
operation: OpenOp,
#[source]
source: sqlx::Error,
},
#[error("migration step {step} (version {version:?}) failed: {source}")]
Migration {
version: Option<i64>,
step: MigrationStep,
#[source]
source: sqlx::Error,
},
#[error("query failed during {operation}: {source}")]
Query {
operation: Cow<'static, str>,
#[source]
source: sqlx::Error,
},
#[error("{entity} not found: '{key}'")]
NotFound { entity: Entity, key: String },
#[error("invalid {entity} reference: {reason}")]
InvalidReference {
entity: Entity,
reason: Cow<'static, str>,
},
#[error("filesystem operation failed at '{}': {source}", path.display())]
Filesystem {
path: PathBuf,
#[source]
source: std::io::Error,
},
}
impl Error {
/// Discriminator for the boundary conversion into [`MagnotiaError`].
pub fn kind(&self) -> StorageKind {
match self {
Error::DatabaseOpen { .. } => StorageKind::DatabaseOpen,
Error::Migration { .. } => StorageKind::Migration,
Error::Query { .. } => StorageKind::Query,
Error::NotFound { .. } => StorageKind::NotFound,
Error::InvalidReference { .. } => StorageKind::InvalidReference,
Error::Filesystem { .. } => StorageKind::Filesystem,
}
}
/// Operation label that flows into the [`MagnotiaError::Storage`] boundary
/// shape. For per-query failures this is the caller-provided label;
/// for the other variants it's the variant's intrinsic label.
pub fn operation_label(&self) -> Cow<'static, str> {
match self {
Error::DatabaseOpen { operation, .. } => Cow::Borrowed(operation.label()),
Error::Migration { step, .. } => Cow::Borrowed(step.label()),
Error::Query { operation, .. } => operation.clone(),
Error::NotFound { entity, .. } => Cow::Owned(format!("not_found:{}", entity.label())),
Error::InvalidReference { entity, .. } => {
Cow::Owned(format!("invalid_reference:{}", entity.label()))
}
Error::Filesystem { .. } => Cow::Borrowed("filesystem"),
}
}
}
/// Boundary conversion — flattens the typed error into the wire-friendly
/// [`MagnotiaError::Storage`] variant. Lives in the storage crate (not in core)
/// to avoid a `core -> storage` dependency cycle.
impl From<Error> for MagnotiaError {
fn from(error: Error) -> Self {
let kind = error.kind();
let operation = error.operation_label().into_owned();
let detail = error.to_string();
MagnotiaError::Storage {
kind,
operation,
detail,
}
}
}
/// `Result` alias for storage-crate functions.
pub type Result<T> = std::result::Result<T, Error>;