Skip to content

refactor(edda-ledger): wrap sqlite_store types in domain abstractions (GH-378)#385

Merged
fagemx merged 3 commits intomainfrom
feat/378-domain-abstractions
Mar 27, 2026
Merged

refactor(edda-ledger): wrap sqlite_store types in domain abstractions (GH-378)#385
fagemx merged 3 commits intomainfrom
feat/378-domain-abstractions

Conversation

@fagemx
Copy link
Copy Markdown
Owner

@fagemx fagemx commented Mar 27, 2026

Summary

  • Phase 1: Extract 9 domain-shaped types (PatternType, DetectedPattern, VillageStats, etc.) from sqlite_store/types.rs to new domain.rs module
  • Phase 2: Migrate 10 Ledger methods from returning DecisionRow to DecisionView, add ChainEntryView domain type, fix edda-ask to derive is_active from status instead of raw boolean, remove redundant to_view() in edda-pack
  • Phase 3: Move remaining public types (BundleRow, DeviceTokenRow, DecideSnapshotRow, SuggestionRow, TaskBriefRow, ImportParams) to domain.rs, rename DepRow to DependencyEdge, seal sqlite_store module as pub(crate)

Key Changes

  • sqlite_store module is now pub(crate) — no downstream crate can import edda_ledger::sqlite_store::* directly
  • All public types are re-exported from lib.rs via domain.rs
  • DecisionRow and ChainEntry remain internal to edda-ledger
  • Internal sync.rs bypasses the Ledger facade to access raw rows for source_project_id checking
  • verify_chain method annotated with #[cfg_attr(not(test), allow(dead_code))] since it's only used in tests

Test plan

  • cargo check passes for all workspace crates
  • cargo clippy -D warnings passes for all affected crates
  • cargo fmt --check passes
  • cargo test — 106 tests pass across edda-ledger, edda-ask, edda-pack, edda-serve, edda-aggregate, edda-ingestion

Closes #378

🤖 Generated with Claude Code

fagemx and others added 3 commits March 27, 2026 18:11
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) <noreply@anthropic.com>
…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) <noreply@anthropic.com>
…-378)

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) <noreply@anthropic.com>
@fagemx
Copy link
Copy Markdown
Owner Author

fagemx commented Mar 27, 2026

Code Review: PR #385 (Round 1) — LGTM 🎉

All P0 and P1 issues have been resolved.

Summary

This PR cleanly wraps sqlite_store types in domain abstractions:

  • sqlite_store sealed as pub(crate) — external crates can no longer access storage internals
  • New domain.rs module — all public types (BundleRow, DependencyEdge, VillageStats, etc.) moved from sqlite_store/types.rs to domain.rs and re-exported from lib.rs
  • Ledger API migrated from DecisionRow to DecisionView — all public methods on Ledger now return DecisionView, with to_view() conversion applied at the Ledger boundary
  • ChainEntry replaced by ChainEntryView in the public API, using DecisionView instead of DecisionRow
  • DepRowDependencyEdge rename with backwards-compatible type alias inside sqlite_store
  • sync.rs correctly uses pub(crate) sqlite field for internal access where raw DecisionRow fields (scope, source_project_id) are needed
  • is_active derivation via matches!(status, "active" | "experimental") is semantically equivalent to the DB column value (verified against status_to_is_active in sqlite_store/mod.rs)

Verification

  • cargo check --workspace — clean
  • cargo clippy --workspace -- -D warnings — clean
  • cargo fmt --check — clean
  • cargo test -p edda-ledger — 159 tests passed
  • cargo test -p edda-ask -p edda-pack -p edda-serve — all tests passed
  • No external crate references edda_ledger::sqlite_store, DecisionRow, DepRow, or ChainEntry
  • All re-exports from lib.rs verified intact

Verdict: LGTM ✅

No critical or high-priority issues remaining. This PR is ready for merge.


Completed after 1 round(s) of automated review-fix loop

@fagemx fagemx merged commit 3b30202 into main Mar 27, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(ledger): wrap sqlite_store types in domain abstractions

1 participant