Feature/unified entity schema docs - #155
Conversation
…atterns Address review comments from Qodo: - Fix tenant scoping: add org_id to ProjectRepository trait (row-level isolation) - Fix file paths: ports in mcb-domain not mcb-application, providers use database/ layout - Fix schema example: use canonical table!/col!/index!/fk!/unique! macros - Include all real fields: org_id, path, updated_at in entity and schema - Add indexes, foreign_keys, unique_constraints to schema example
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
PR Summary by QodoDocument clean architecture and unified domain schemas
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
36 rules 1. Nonexistent application layer
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| // mcb-providers/src/persistence/sqlite/project_repository.rs | ||
| // Adapter maps Project <-> SQL rows and uses SchemaDdlGenerator for backend DDL. |
There was a problem hiding this comment.
1. Nonexistent provider path 🐞 Bug ⚙ Maintainability
The doc’s “after” example points readers to mcb-providers/src/persistence/sqlite/project_repository.rs, but the SQLite ProjectRepository implementation in this repo is under crates/mcb-providers/src/database/sqlite/project_repository.rs. This makes the example hard to follow and encourages editing/creating the wrong module path.
Agent Prompt
## Issue description
The docs reference a provider file path that doesn’t exist in this repository, which misleads readers.
## Issue Context
The “after” example currently points to `mcb-providers/src/persistence/sqlite/project_repository.rs`, but the actual SQLite implementation is located under `crates/mcb-providers/src/database/sqlite/project_repository.rs`.
## Fix Focus Areas
- docs/architecture/CLEAN_ARCHITECTURE.md[218-220]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| // mcb-domain/src/entities/project.rs (incomplete — missing org_id, path, updated_at) | ||
| pub struct Project { | ||
| pub id: String, | ||
| pub name: String, | ||
| } | ||
|
|
||
| // mcb-providers/src/database/sqlite/project_repository.rs (leaks storage type) | ||
| pub struct ProjectRow { | ||
| pub id: String, | ||
| pub name: String, | ||
| pub created_at: i64, | ||
| } | ||
|
|
||
| // mcb-domain/src/ports/repositories/project.rs (trait uses storage type, no tenant scoping) | ||
| #[async_trait] | ||
| pub trait ProjectRepo { | ||
| async fn save(&self, row: ProjectRow) -> anyhow::Result<()>; | ||
| async fn get(&self, id: &str) -> anyhow::Result<Option<ProjectRow>>; | ||
| } |
There was a problem hiding this comment.
2. Pseudo-code uses real paths 🐞 Bug ⚙ Maintainability
The “Before” snippet uses real repo paths (mcb-domain/src/entities/project.rs, mcb-providers/src/database/sqlite/project_repository.rs, mcb-domain/src/ports/repositories/project.rs) while claiming shapes/traits that don’t match the current code, making it look like an inaccurate description of the actual codebase. This should be clearly labeled as illustrative (or updated to match real files) to avoid confusing contributors about the current architecture.
Agent Prompt
## Issue description
The “Before” example appears to describe real code (it uses real repo file paths), but the structs/traits shown don’t match what is currently implemented. This reads like a factual statement about the repository and can mislead contributors.
## Issue Context
- `Project` in the domain already contains `org_id`, `path`, and `updated_at`.
- The SQLite provider repository uses `Project` directly (no `ProjectRow`).
- The real `ProjectRepository` trait includes `get_by_path`, which the example omits.
## Fix Focus Areas
- docs/architecture/CLEAN_ARCHITECTURE.md[133-155]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| 1. Keep the **canonical model** in `mcb-domain` as the single source of truth. | ||
| 2. Express persistence metadata through domain schema types (`Schema`, `TableDef`, `ColumnDef`). | ||
| 3. Keep serialization/transport and DB-specific concerns in outer layers (providers/server). | ||
| 4. Generate backend-specific DDL via `SchemaDdlGenerator` implementations in adapters. |
There was a problem hiding this comment.
3. Domain serialization guidance conflict 🐞 Bug ⚙ Maintainability
The new guidance says to keep serialization/transport concerns in outer layers, but the domain Project entity currently depends on serde/schemars via derives. The doc should either document this as an intentional exception/allowed pattern or refine the guidance so it matches current practice.
Agent Prompt
## Issue description
The docs recommend keeping serialization/transport concerns out of the domain, but current domain entities derive serde/schemars, making the guidance inconsistent with the codebase.
## Issue Context
`Project` in `mcb-domain` imports `serde` and `schemars` and derives `Serialize`/`Deserialize`.
## Fix Focus Areas
- docs/architecture/CLEAN_ARCHITECTURE.md[120-127]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit f828ea9 |
| │ Layer 4: Application Services & Use Cases │ | ||
| │ (orchestration, business logic, registry) │ | ||
| │ Crate: mcb-application │ |
There was a problem hiding this comment.
1. Nonexistent application layer 🐞 Bug ⚙ Maintainability
The layer model assigns services and registries to an mcb-application crate that is not a workspace member, so contributors cannot place or import code as documented. The actual seven-crate workspace includes mcb-utils and mcb-validate, which this model omits.
Agent Prompt
## Issue description
The architecture model introduces a nonexistent `mcb-application` crate while omitting actual workspace crates. Update the document to use the repository's real crate ownership and dependency structure.
## Issue Context
The workspace and boundary documentation define seven crates: `mcb`, `mcb-utils`, `mcb-domain`, `mcb-providers`, `mcb-infrastructure`, `mcb-server`, and `mcb-validate`.
## Fix Focus Areas
- docs/architecture/CLEAN_ARCHITECTURE.md[6-46]
- docs/architecture/CLEAN_ARCHITECTURE.md[229-267]
- Cargo.toml[1-11]
- docs/architecture/ARCHITECTURE_BOUNDARIES.md[54-74]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| pub async fn build_catalog(config: AppConfig) -> Result<Catalog> { | ||
| CatalogBuilder::new() | ||
| .add_value(config) | ||
| .add_value(embedding_provider) | ||
| .add_value(embedding_handle) | ||
| .add_value(embedding_admin) | ||
| .build() |
There was a problem hiding this comment.
2. Removed di container documented 🐞 Bug ⚙ Maintainability
The infrastructure instructions tell contributors to register services through dill's CatalogBuilder, but dill and build_catalog were removed in favor of explicit bootstrap wiring. Following this example or the new-service recipe therefore references APIs that no longer exist.
Agent Prompt
## Issue description
Replace the obsolete dill/CatalogBuilder examples with the current Loco and manual bootstrap composition flow.
## Issue Context
ADR-050 superseded dill and records that `build_catalog()` was never used and was removed. Current composition occurs through the MCP initializer's bootstrap functions.
## Fix Focus Areas
- docs/architecture/CLEAN_ARCHITECTURE.md[273-332]
- docs/architecture/CLEAN_ARCHITECTURE.md[503-516]
- docs/adr/050-manual-composition-root-dill-removal.md[20-42]
- crates/mcb/src/initializers/mcp_server.rs[271-305]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| 2. Express persistence metadata through domain schema types (`Schema`, `TableDef`, `ColumnDef`). | ||
| 3. Keep serialization/transport and DB-specific concerns in outer layers (providers/server). | ||
| 4. Generate backend-specific DDL via `SchemaDdlGenerator` implementations in adapters. |
There was a problem hiding this comment.
3. Obsolete schema framework prescribed 🐞 Bug ⚙ Maintainability
The recommended pattern directs contributors to use Schema, TableDef, ColumnDef, and SchemaDdlGenerator, but the current domain crate exposes no schema module or these declarations. Adopting the example would require resurrecting a removed framework instead of extending the current SeaORM entities and migrations.
Agent Prompt
## Issue description
Remove or clearly mark the proposed domain-schema framework as a future design, and document the schema mechanism currently implemented in the repository.
## Issue Context
`mcb-domain` exports no schema module. Current persistence shape is represented by provider-layer SeaORM entities and migrations, while validation still recognizes the old root schema symbols as forbidden legacy paths.
## Fix Focus Areas
- docs/architecture/CLEAN_ARCHITECTURE.md[112-127]
- docs/architecture/CLEAN_ARCHITECTURE.md[176-201]
- crates/mcb-domain/src/lib.rs[60-81]
- crates/mcb-providers/src/database/seaorm/entities/projects.rs[1-32]
- crates/mcb-providers/src/database/seaorm/migration/m20260301_000001_initial_schema.rs[11-27]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| factory: || { | ||
| Ok(Arc::new(OllamaEmbeddingProvider::new()?)) | ||
| }, |
There was a problem hiding this comment.
4. Invalid provider registration recipe 🐞 Bug ≡ Correctness
The extension recipe registers a zero-argument closure that calls OllamaEmbeddingProvider::new() without configuration, but registry factories accept &EmbeddingProviderConfig and the Ollama constructor requires URL, model, timeout, and client arguments. Copying this template will not type-check and cannot consume runtime provider configuration.
Agent Prompt
## Issue description
Rewrite the provider registration examples to use the current configuration-aware factory signature and registration macro.
## Issue Context
Embedding factories receive `&EmbeddingProviderConfig`. The existing Ollama implementation resolves its configuration in a named factory function and registers that function through `register_embedding_provider!`.
## Fix Focus Areas
- docs/architecture/CLEAN_ARCHITECTURE.md[365-373]
- docs/architecture/CLEAN_ARCHITECTURE.md[464-472]
- crates/mcb-domain/src/registry/embedding.rs[13-56]
- crates/mcb-providers/src/embedding/ollama.rs[173-200]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 772a5d6 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
12 issues found across 1 file
Confidence score: 3/5
docs/architecture/CLEAN_ARCHITECTURE.mdcurrently describes a canonical schema, application layer (mcb-application), and DI flow (CatalogBuilder/build_catalog()) that do not exist, so contributors following it can be led into dead-end implementations and duplicate architecture patterns—realign these sections to the currentAppContextcomposition root and existing workspace crates.docs/architecture/CLEAN_ARCHITECTURE.mdhas a reversed dependency graph and incorrect domain boundary claims, which can cause reviewers and contributors to enforce the wrong layering decisions in future changes—update diagrams/rules to match Cargo manifests and ADR-050 as the authoritative boundaries.docs/architecture/CLEAN_ARCHITECTURE.mdreferences nonexistent public API symbols (for examplemcb_infrastructure::AppContext) and mismatched port/trait contracts (VectorStoreProvider,EmbeddingProvider), so copy-pasted examples will not compile and extension work may stall—replace snippets with currentMcbApp/McpServerexports and exact trait method sets.docs/architecture/CLEAN_ARCHITECTURE.mdappears to replace a stable-link page with a second, already-stale full architecture spec, creating two competing sources of truth that will drift further—keep normative architecture detail in ADRs and convert this page to a pointer/synopsis to reduce future regression risk.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/architecture/CLEAN_ARCHITECTURE.md">
<violation number="1" location="docs/architecture/CLEAN_ARCHITECTURE.md:6">
P2: Replacing the stable-link page with a second full architecture specification creates a competing source of truth, and the copy is already stale relative to Cargo manifests and ADR-050. Keeping the normative detail in `ARCHITECTURE.md`/`PATTERNS.md` and limiting this file to focused unified-schema material plus links would avoid maintaining two divergent architecture descriptions.</violation>
<violation number="2" location="docs/architecture/CLEAN_ARCHITECTURE.md:26">
P1: The documented application layer cannot be followed because `mcb-application` is not a workspace crate. This also makes the service, registry, dependency, and extension guidance that follows point contributors at a nonexistent location. The layer should be mapped to the actual seven-crate workspace and its real service/registry locations.</violation>
<violation number="3" location="docs/architecture/CLEAN_ARCHITECTURE.md:66">
P2: This public-API example references a nonexistent `mcb_infrastructure::AppContext`, so users copying it or looking for that facade export will fail. Replace it with the actual `McbApp`/`McpServer` exports or clearly label a proposed API.</violation>
<violation number="4" location="docs/architecture/CLEAN_ARCHITECTURE.md:71">
P3: The layer numbering is internally inconsistent: the overview calls domain Layer 3, while this heading calls it Layer 2 (and providers are Layer 2 in the overview but Layer 5 later). Aligning all headings with the overview—or removing ordinal numbers—would make layer references unambiguous.</violation>
<violation number="5" location="docs/architecture/CLEAN_ARCHITECTURE.md:96">
P2: The documented `VectorStoreProvider` contract does not match the actual port, so the `ContextService` example calling `search` cannot compile. Use the real collection-aware methods or mark the snippet as pseudocode.</violation>
<violation number="6" location="docs/architecture/CLEAN_ARCHITECTURE.md:110">
P2: The stated domain dependency boundary is false and may cause contributors to reject dependencies that are already part of the enforced architecture. Reference the manifest or accurately list the allowed dependency categories.</violation>
<violation number="7" location="docs/architecture/CLEAN_ARCHITECTURE.md:123">
P1: The “canonical schema” recommendation describes an API and persistence architecture that do not exist in this repository. A contributor following it cannot compile the example and would create a second schema source alongside the implemented SeaORM entities/migrations. This section should document the current entity-first SeaORM flow, or be explicitly marked as a proposal and introduced together with the referenced schema APIs.</violation>
<violation number="8" location="docs/architecture/CLEAN_ARCHITECTURE.md:275">
P2: The DI guidance has reverted to the removed dill/Catalog architecture. ADR-050 records that `CatalogBuilder` and `build_catalog()` were deleted in favor of the manual `AppContext` composition root, so this example sends contributors toward unavailable APIs. The infrastructure responsibilities and examples should describe the current manual wiring plus linkme discovery.</violation>
<violation number="9" location="docs/architecture/CLEAN_ARCHITECTURE.md:432">
P2: The dependency graph reverses the facade edge: the actual `mcb` facade depends on the internal crates; `mcb-domain` does not depend on `mcb`. Because this diagram is paired with the “Dependencies flow INWARD” rule, it currently teaches a dependency that would violate the enforced domain boundary. The arrows should match the Cargo manifests and treat the facade as an outer composition/entry crate.</violation>
<violation number="10" location="docs/architecture/CLEAN_ARCHITECTURE.md:457">
P2: The “Adding a New Embedding Provider” implementation does not satisfy the current `EmbeddingProvider` trait. Include the three required methods so the extension pattern is copyable.</violation>
<violation number="11" location="docs/architecture/CLEAN_ARCHITECTURE.md:479">
P2: This extension example does not compile because `switch_to_my_provider` returns `()` while using `?`. Returning `Result<()>` and completing with `Ok(())` makes the documented usage valid.</violation>
<violation number="12" location="docs/architecture/CLEAN_ARCHITECTURE.md:527">
P2: The new-service handler example returns `Result<()>` from `do_something()` where its signature promises `Result<MyToolResponse>`, so it cannot compile. The example should await the service call, construct the documented response, and return `Ok(response)` (or align the handler signature with the service result if no response payload is intended).</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| Recommended pattern: | ||
|
|
||
| 1. Keep the **canonical model** in `mcb-domain` as the single source of truth. | ||
| 2. Express persistence metadata through domain schema types (`Schema`, `TableDef`, `ColumnDef`). |
There was a problem hiding this comment.
P1: The “canonical schema” recommendation describes an API and persistence architecture that do not exist in this repository. A contributor following it cannot compile the example and would create a second schema source alongside the implemented SeaORM entities/migrations. This section should document the current entity-first SeaORM flow, or be explicitly marked as a proposal and introduced together with the referenced schema APIs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/architecture/CLEAN_ARCHITECTURE.md, line 123:
<comment>The “canonical schema” recommendation describes an API and persistence architecture that do not exist in this repository. A contributor following it cannot compile the example and would create a second schema source alongside the implemented SeaORM entities/migrations. This section should document the current entity-first SeaORM flow, or be explicitly marked as a proposal and introduced together with the referenced schema APIs.</comment>
<file context>
@@ -1,18 +1,621 @@
+Recommended pattern:
+
+1. Keep the **canonical model** in `mcb-domain` as the single source of truth.
+2. Express persistence metadata through domain schema types (`Schema`, `TableDef`, `ColumnDef`).
+3. Keep serialization/transport and DB-specific concerns in outer layers (providers/server).
+4. Generate backend-specific DDL via `SchemaDdlGenerator` implementations in adapters.
</file context>
| ┌─────────────────────────────────────────────────────────────┐ | ||
| │ Layer 4: Application Services & Use Cases │ | ||
| │ (orchestration, business logic, registry) │ | ||
| │ Crate: mcb-application │ |
There was a problem hiding this comment.
P1: The documented application layer cannot be followed because mcb-application is not a workspace crate. This also makes the service, registry, dependency, and extension guidance that follows point contributors at a nonexistent location. The layer should be mapped to the actual seven-crate workspace and its real service/registry locations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/architecture/CLEAN_ARCHITECTURE.md, line 26:
<comment>The documented application layer cannot be followed because `mcb-application` is not a workspace crate. This also makes the service, registry, dependency, and extension guidance that follows point contributors at a nonexistent location. The layer should be mapped to the actual seven-crate workspace and its real service/registry locations.</comment>
<file context>
@@ -1,18 +1,621 @@
+┌─────────────────────────────────────────────────────────────┐
+│ Layer 4: Application Services & Use Cases │
+│ (orchestration, business logic, registry) │
+│ Crate: mcb-application │
+└─────────────────────────────────────────────────────────────┘
+ ↓
</file context>
| } | ||
|
|
||
| pub trait VectorStoreProvider: Send + Sync { | ||
| async fn store(&self, embedding: Embedding) -> Result<()>; |
There was a problem hiding this comment.
P2: The documented VectorStoreProvider contract does not match the actual port, so the ContextService example calling search cannot compile. Use the real collection-aware methods or mark the snippet as pseudocode.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/architecture/CLEAN_ARCHITECTURE.md, line 96:
<comment>The documented `VectorStoreProvider` contract does not match the actual port, so the `ContextService` example calling `search` cannot compile. Use the real collection-aware methods or mark the snippet as pseudocode.</comment>
<file context>
@@ -1,18 +1,621 @@
+}
+
+pub trait VectorStoreProvider: Send + Sync {
+ async fn store(&self, embedding: Embedding) -> Result<()>;
+ async fn search(&self, query: &Embedding) -> Result<Vec<SearchResult>>;
+}
</file context>
| } | ||
|
|
||
| #[async_trait] | ||
| impl EmbeddingProvider for MyEmbeddingProvider { |
There was a problem hiding this comment.
P2: The “Adding a New Embedding Provider” implementation does not satisfy the current EmbeddingProvider trait. Include the three required methods so the extension pattern is copyable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/architecture/CLEAN_ARCHITECTURE.md, line 457:
<comment>The “Adding a New Embedding Provider” implementation does not satisfy the current `EmbeddingProvider` trait. Include the three required methods so the extension pattern is copyable.</comment>
<file context>
@@ -1,18 +1,621 @@
+}
+
+#[async_trait]
+impl EmbeddingProvider for MyEmbeddingProvider {
+ async fn embed(&self, text: &str) -> Result<Embedding> {
+ // Implementation
</file context>
| } | ||
| ``` | ||
|
|
||
| **Dependency**: None (except standard library + thiserror) |
There was a problem hiding this comment.
P2: The stated domain dependency boundary is false and may cause contributors to reject dependencies that are already part of the enforced architecture. Reference the manifest or accurately list the allowed dependency categories.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/architecture/CLEAN_ARCHITECTURE.md, line 110:
<comment>The stated domain dependency boundary is false and may cause contributors to reject dependencies that are already part of the enforced architecture. Reference the manifest or accurately list the allowed dependency categories.</comment>
<file context>
@@ -1,18 +1,621 @@
+}
+```
+
+**Dependency**: None (except standard library + thiserror)
+
+#### Unified Domain Data Model (Entities + Schemas + Traits)
</file context>
| pub async fn switch_to_my_provider(admin: &dyn EmbeddingAdminInterface) { | ||
| admin.switch_provider("my_provider").await?; | ||
| } |
There was a problem hiding this comment.
P2: This extension example does not compile because switch_to_my_provider returns () while using ?. Returning Result<()> and completing with Ok(()) makes the documented usage valid.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/architecture/CLEAN_ARCHITECTURE.md, line 479:
<comment>This extension example does not compile because `switch_to_my_provider` returns `()` while using `?`. Returning `Result<()>` and completing with `Ok(())` makes the documented usage valid.</comment>
<file context>
@@ -1,18 +1,621 @@
+
+```rust
+// Automatically discovered and available for switching
+pub async fn switch_to_my_provider(admin: &dyn EmbeddingAdminInterface) {
+ admin.switch_provider("my_provider").await?;
+}
</file context>
| pub async fn switch_to_my_provider(admin: &dyn EmbeddingAdminInterface) { | |
| admin.switch_provider("my_provider").await?; | |
| } | |
| pub async fn switch_to_my_provider(admin: &dyn EmbeddingAdminInterface) -> Result<()> { | |
| admin.switch_provider("my_provider").await?; | |
| Ok(()) | |
| } |
| @@ -1,18 +1,621 @@ | |||
| <!-- markdownlint-disable MD013 MD024 MD025 MD003 MD022 MD031 MD032 MD036 MD041 MD060 --> | |||
There was a problem hiding this comment.
P2: Replacing the stable-link page with a second full architecture specification creates a competing source of truth, and the copy is already stale relative to Cargo manifests and ADR-050. Keeping the normative detail in ARCHITECTURE.md/PATTERNS.md and limiting this file to focused unified-schema material plus links would avoid maintaining two divergent architecture descriptions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/architecture/CLEAN_ARCHITECTURE.md, line 6:
<comment>Replacing the stable-link page with a second full architecture specification creates a competing source of truth, and the copy is already stale relative to Cargo manifests and ADR-050. Keeping the normative detail in `ARCHITECTURE.md`/`PATTERNS.md` and limiting this file to focused unified-schema material plus links would avoid maintaining two divergent architecture descriptions.</comment>
<file context>
@@ -1,18 +1,621 @@
+
+## Overview
+
+Memory Context Browser follows **Clean Architecture** principles with strict layer separation across 7 Cargo workspace crates. This document explains the architecture, layer interactions, and extension patterns.
+
+## The 6 Layers
</file context>
| ↓ | ||
| mcb-domain | ||
| ↓ | ||
| mcb (facade) |
There was a problem hiding this comment.
P2: The dependency graph reverses the facade edge: the actual mcb facade depends on the internal crates; mcb-domain does not depend on mcb. Because this diagram is paired with the “Dependencies flow INWARD” rule, it currently teaches a dependency that would violate the enforced domain boundary. The arrows should match the Cargo manifests and treat the facade as an outer composition/entry crate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/architecture/CLEAN_ARCHITECTURE.md, line 432:
<comment>The dependency graph reverses the facade edge: the actual `mcb` facade depends on the internal crates; `mcb-domain` does not depend on `mcb`. Because this diagram is paired with the “Dependencies flow INWARD” rule, it currently teaches a dependency that would violate the enforced domain boundary. The arrows should match the Cargo manifests and treat the facade as an outer composition/entry crate.</comment>
<file context>
@@ -1,18 +1,621 @@
+ ↓
+mcb-domain
+ ↓
+mcb (facade)
+```
+
</file context>
|
|
||
| #### Responsibilities | ||
|
|
||
| - Build DI container (dill Catalog) |
There was a problem hiding this comment.
P2: The DI guidance has reverted to the removed dill/Catalog architecture. ADR-050 records that CatalogBuilder and build_catalog() were deleted in favor of the manual AppContext composition root, so this example sends contributors toward unavailable APIs. The infrastructure responsibilities and examples should describe the current manual wiring plus linkme discovery.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/architecture/CLEAN_ARCHITECTURE.md, line 275:
<comment>The DI guidance has reverted to the removed dill/Catalog architecture. ADR-050 records that `CatalogBuilder` and `build_catalog()` were deleted in favor of the manual `AppContext` composition root, so this example sends contributors toward unavailable APIs. The infrastructure responsibilities and examples should describe the current manual wiring plus linkme discovery.</comment>
<file context>
@@ -1,18 +1,621 @@
+
+#### Responsibilities
+
+- Build DI container (dill Catalog)
+- Load configuration (Figment)
+- Provide provider handles (RwLock wrappers for runtime switching)
</file context>
|
|
||
| **Dependency**: Imports from all other crates (but only re-exports public types) | ||
|
|
||
| ### Layer 2: Domain (mcb-domain) |
There was a problem hiding this comment.
P3: The layer numbering is internally inconsistent: the overview calls domain Layer 3, while this heading calls it Layer 2 (and providers are Layer 2 in the overview but Layer 5 later). Aligning all headings with the overview—or removing ordinal numbers—would make layer references unambiguous.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/architecture/CLEAN_ARCHITECTURE.md, line 71:
<comment>The layer numbering is internally inconsistent: the overview calls domain Layer 3, while this heading calls it Layer 2 (and providers are Layer 2 in the overview but Layer 5 later). Aligning all headings with the overview—or removing ordinal numbers—would make layer references unambiguous.</comment>
<file context>
@@ -1,18 +1,621 @@
+
+**Dependency**: Imports from all other crates (but only re-exports public types)
+
+### Layer 2: Domain (mcb-domain)
+
+**Purpose**: Business rules and domain entities
</file context>
|
Fechando como superseded pela SSOT atual na main. Evidence: Nenhum conteúdo único identificado no diff que já não esteja coberto pela SSOT atual. Se houver seção específica que deva migrar para |
2 participants
No description provided.