riotpiao-poimen/poimen-memory
2
0
Fork 0
Code Issues 13 Pull Requests Actions Packages Projects Releases Wiki Activity

[Phase 3.1] Agent entity types + metadata structs #47

Merged
rock merged 2 commits from feat/phase-3.1-agent-prompt-entity into main 2026-09-09 02:48:43 +00:00
Conversation 1 Commits 2 Files Changed 5
+364 -8
rock commented 2026-09-08 23:59:09 +00:00
Owner
Copy Link
Copy Source

Changes

  • crates/mem-core/src/entity.rs — Added AgentPrompt, AgentSkill, AgentDecision to EntityType enum
  • crates/mem-core/src/agent_entity.rs — New module (280 LOC): metadata structs, factories, stat updaters
  • crates/mem-core/src/lib.rs — Module registration + exports

Agent Entity Types

  • AgentPrompt: template, target_model, task_category, usage_count, avg_quality, version
  • AgentSkill: description, trigger_patterns, success_rate, invocation_count, avg_latency_ms
  • AgentDecision: action, reasoning, alternatives, confidence, outcome (success/quality/feedback)

Validation

  • 8 new tests pass (factories, stats, round-trip, serialization)
  • 174 total lib tests pass
  • cargo build --release clean
## Changes - `crates/mem-core/src/entity.rs` — Added AgentPrompt, AgentSkill, AgentDecision to EntityType enum - `crates/mem-core/src/agent_entity.rs` — New module (280 LOC): metadata structs, factories, stat updaters - `crates/mem-core/src/lib.rs` — Module registration + exports ## Agent Entity Types - **AgentPrompt**: template, target_model, task_category, usage_count, avg_quality, version - **AgentSkill**: description, trigger_patterns, success_rate, invocation_count, avg_latency_ms - **AgentDecision**: action, reasoning, alternatives, confidence, outcome (success/quality/feedback) ## Validation - 8 new tests pass (factories, stats, round-trip, serialization) - 174 total lib tests pass - `cargo build --release` clean
rock force-pushed feat/phase-3.1-agent-prompt-entity from 1e86ae655f to 1151c421f9 2026-09-09 00:08:26 +00:00 Compare
rock force-pushed feat/phase-3.1-agent-prompt-entity from 1151c421f9 to d281fc8036 2026-09-09 01:09:33 +00:00 Compare
rock force-pushed feat/phase-3.1-agent-prompt-entity from 56438e7751 to 2f7d825ca5 2026-09-09 02:13:45 +00:00 Compare
poimen commented 2026-09-09 02:17:27 +00:00
Member
Copy Link
Copy Source

CRAP / SOLID / DRY Review — PR #46 + #47

🔴 Critical: 198 Unit Tests Deleted in PR #46

PR #46 removed all #[cfg(test)] modules from 13 files instead of fixing compilation errors. This leaves zero test coverage on core modules:

Module Tests Before Tests Now Risk
http_server.rs 10 0 CRITICAL — 1,408 LOC, 236-LOC god function
inference_engine.rs 28 0 HIGH — reasoning path confidence
query_reasoner.rs 28 0 HIGH — query reasoning logic
faceted_search.rs 24 0 HIGH — search faceting
entity_linker.rs 18 0 HIGH — entity resolution
observability.rs 18 0 MEDIUM — metrics collection
community_detector.rs 13 0 MEDIUM — graph community detection
synthesis.rs 11 0 MEDIUM — synthesis handler
semantic.rs 9 0 MEDIUM — semantic search
agent_handler.rs 9 0 MEDIUM — agent CRUD
parallel_dual_write.rs 9 0 MEDIUM — dual-write consistency
query_router.rs 11 0 MEDIUM — query routing
semantic_retriever.rs 10 0 MEDIUM — retrieval pipeline

Total: 198 tests deleted, 0 replaced.

Root Causes (fixable, should not have been deleted)

  1. Float ambiguity (6 files) — fix: add f32/f64 type annotations to vec![] declarations
  2. Missing struct fields (3 files) — fix: add new fields to test constructors with defaults
  3. Private fields (1 file) — fix: use constructor or pub(crate)
  4. RBAC import path (1 file) — fix: add use crate::rbac::types::*
  5. build_lazy API change (3 files) — fix: use current sqlx::PgPool::connect() or mock

Action Required: Create follow-up PR to restore all 198 tests with fixes. NOT optional — this is Rust, the whole point is compile-time safety + test coverage.

CRAP Analysis (Complexity-Risk-Anti-Pattern)

Module LOC Longest fn CRAP Score Verdict
http_server.rs 1,408 236 (start_server) 42 🔴 FAIL (>30)
handlers/ingest.rs ~600 200 35 🔴 FAIL
handlers/synthesis.rs 734 ~80 22 ✅ OK
query/inference_engine.rs 367 ~60 18 ✅ OK
query/query_reasoner.rs 415 ~50 15 ✅ OK
query/faceted_search.rs 362 ~40 12 ✅ OK

http_server.rs is the worst offender: start_server() at 236 LOC is a god function that creates pool, registers all routes, and configures middleware. Should be split into create_pool(), configure_routes(), build_app().

SOLID Violations

Principle Violation Where Fix
S (Single Responsibility) start_server() does pool creation + route config + middleware + server bind http_server.rs:191 Split into 3-4 functions
S execute_ingest() does validation + extraction + DB write + response http_server.rs:467 Extract validation, DB write
O (Open/Closed) Error responses are hardcoded json!({"error":...}) in 20 places everywhere Create ApiError enum with impl IntoResponse
D (Dependency Inversion) Handlers directly construct sqlx::query!() http_server.rs Use repository trait

DRY Violations

Pattern Count Where Fix
HttpResponse::InternalServerError().json(json!({"error":...})) 20 All handlers ApiError::internal(msg) helper
json!({"error": "...", "reason": e.to_string()}) 10 All handlers Standardized error response struct
Manual JWT extraction + validation 5+ Multiple handlers Extract to middleware/guard
sqlx::query!(...).fetch_all(&state.pool) 15+ http_server.rs Repository pattern

Recommendations (Priority Order)

  1. P0: Restore 198 tests — fix compilation errors, do not delete. Follow-up PR.
  2. P1: Split http_server.rs — extract start_server() into modular pieces. CRAP 42 is unacceptable.
  3. P1: Create ApiError type — eliminates 20+ DRY violations in one PR.
  4. P2: Repository trait for DB queries — decouples handlers from sqlx, enables proper mocking.
  5. P2: Middleware for JWT extraction — eliminates repeated auth boilerplate.

PR #47 Specific Review (Agent Entity Types)

SOLID: ✅ Good separation. agent_entity.rs is self-contained, factory functions follow SRP.
DRY: ✅ No duplication. Factory functions avoid repeated construction.
CRAP: ✅ All functions <15 LOC. Score ~5.
Tests: ✅ 8 tests covering factories, stats, round-trip, serialization.
One concern: record_prompt_usage() and record_skill_invocation() do running average math that can drift with floating point. Consider using total_sum + count instead of avg recalculation.

## CRAP / SOLID / DRY Review — PR #46 + #47 ### 🔴 Critical: 198 Unit Tests Deleted in PR #46 PR #46 removed all `#[cfg(test)]` modules from 13 files instead of fixing compilation errors. This leaves **zero test coverage** on core modules: | Module | Tests Before | Tests Now | Risk | |--------|-------------|-----------|------| | http_server.rs | 10 | 0 | **CRITICAL** — 1,408 LOC, 236-LOC god function | | inference_engine.rs | 28 | 0 | HIGH — reasoning path confidence | | query_reasoner.rs | 28 | 0 | HIGH — query reasoning logic | | faceted_search.rs | 24 | 0 | HIGH — search faceting | | entity_linker.rs | 18 | 0 | HIGH — entity resolution | | observability.rs | 18 | 0 | MEDIUM — metrics collection | | community_detector.rs | 13 | 0 | MEDIUM — graph community detection | | synthesis.rs | 11 | 0 | MEDIUM — synthesis handler | | semantic.rs | 9 | 0 | MEDIUM — semantic search | | agent_handler.rs | 9 | 0 | MEDIUM — agent CRUD | | parallel_dual_write.rs | 9 | 0 | MEDIUM — dual-write consistency | | query_router.rs | 11 | 0 | MEDIUM — query routing | | semantic_retriever.rs | 10 | 0 | MEDIUM — retrieval pipeline | **Total: 198 tests deleted, 0 replaced.** ### Root Causes (fixable, should not have been deleted) 1. **Float ambiguity** (6 files) — fix: add `f32`/`f64` type annotations to `vec![]` declarations 2. **Missing struct fields** (3 files) — fix: add new fields to test constructors with defaults 3. **Private fields** (1 file) — fix: use constructor or `pub(crate)` 4. **RBAC import path** (1 file) — fix: add `use crate::rbac::types::*` 5. **`build_lazy` API change** (3 files) — fix: use current `sqlx::PgPool::connect()` or mock **Action Required**: Create follow-up PR to restore all 198 tests with fixes. NOT optional — this is Rust, the whole point is compile-time safety + test coverage. --- ### CRAP Analysis (Complexity-Risk-Anti-Pattern) | Module | LOC | Longest fn | CRAP Score | Verdict | |--------|-----|-----------|------------|---------| | http_server.rs | 1,408 | 236 (start_server) | **42** | 🔴 FAIL (>30) | | handlers/ingest.rs | ~600 | 200 | **35** | 🔴 FAIL | | handlers/synthesis.rs | 734 | ~80 | 22 | ✅ OK | | query/inference_engine.rs | 367 | ~60 | 18 | ✅ OK | | query/query_reasoner.rs | 415 | ~50 | 15 | ✅ OK | | query/faceted_search.rs | 362 | ~40 | 12 | ✅ OK | **`http_server.rs` is the worst offender**: `start_server()` at 236 LOC is a god function that creates pool, registers all routes, and configures middleware. Should be split into `create_pool()`, `configure_routes()`, `build_app()`. --- ### SOLID Violations | Principle | Violation | Where | Fix | |-----------|-----------|-------|-----| | **S** (Single Responsibility) | `start_server()` does pool creation + route config + middleware + server bind | http_server.rs:191 | Split into 3-4 functions | | **S** | `execute_ingest()` does validation + extraction + DB write + response | http_server.rs:467 | Extract validation, DB write | | **O** (Open/Closed) | Error responses are hardcoded `json!({"error":...})` in 20 places | everywhere | Create `ApiError` enum with `impl IntoResponse` | | **D** (Dependency Inversion) | Handlers directly construct `sqlx::query!()` | http_server.rs | Use repository trait | --- ### DRY Violations | Pattern | Count | Where | Fix | |---------|-------|-------|-----| | `HttpResponse::InternalServerError().json(json!({"error":...}))` | 20 | All handlers | `ApiError::internal(msg)` helper | | `json!({"error": "...", "reason": e.to_string()})` | 10 | All handlers | Standardized error response struct | | Manual JWT extraction + validation | 5+ | Multiple handlers | Extract to middleware/guard | | `sqlx::query!(...).fetch_all(&state.pool)` | 15+ | http_server.rs | Repository pattern | --- ### Recommendations (Priority Order) 1. **P0: Restore 198 tests** — fix compilation errors, do not delete. Follow-up PR. 2. **P1: Split http_server.rs** — extract `start_server()` into modular pieces. CRAP 42 is unacceptable. 3. **P1: Create `ApiError` type** — eliminates 20+ DRY violations in one PR. 4. **P2: Repository trait for DB queries** — decouples handlers from sqlx, enables proper mocking. 5. **P2: Middleware for JWT extraction** — eliminates repeated auth boilerplate. --- ### PR #47 Specific Review (Agent Entity Types) **SOLID**: ✅ Good separation. `agent_entity.rs` is self-contained, factory functions follow SRP. **DRY**: ✅ No duplication. Factory functions avoid repeated construction. **CRAP**: ✅ All functions <15 LOC. Score ~5. **Tests**: ✅ 8 tests covering factories, stats, round-trip, serialization. **One concern**: `record_prompt_usage()` and `record_skill_invocation()` do running average math that can drift with floating point. Consider using total_sum + count instead of avg recalculation.
rock force-pushed feat/phase-3.1-agent-prompt-entity from 2f7d825ca5 to dee7744e45 2026-09-09 02:18:08 +00:00 Compare
rock added 1 commit 2026-09-09 02:23:49 +00:00
feat(phase-3.1): agent entity types + metadata structs 050b3625dc 
EntityType enum extended with 3 agent types:
  - AgentPrompt: track prompt templates, usage, quality
  - AgentSkill: track learned capabilities, success rate, latency
  - AgentDecision: track decisions, reasoning, outcomes

New module: agent_entity.rs (280 LOC)
  Structs: AgentPromptMeta, AgentSkillMeta, AgentDecisionMeta, DecisionOutcome
  Factories: new_agent_prompt(), new_agent_skill(), new_agent_decision()
  Updaters: record_prompt_usage(), record_skill_invocation(), record_decision_outcome()
  Exports: added to mem-core lib.rs

Tests: 8 new (prompt, skill, decision, outcome, usage stats,
       invocation stats, round-trip, serialization)
Build: cargo build --release clean
Suite: 174 lib tests pass
rock force-pushed feat/phase-3.1-agent-prompt-entity from dee7744e45 to fe3b84df0d 2026-09-09 02:23:49 +00:00 Compare
rock added 1 commit 2026-09-09 02:26:47 +00:00
ci: add deploy workflow — retag SHA as latest on main merge
CI / CI (pull_request) Successful in 11m29s
Details
77d2bc1807 
deploy.yaml (main push only):
  - Pull image by SHA tag (already pushed during PR CI)
  - Tag as :latest and push
  - No rebuild needed
rock force-pushed feat/phase-3.1-agent-prompt-entity from fe3b84df0d to 77d2bc1807 2026-09-09 02:26:47 +00:00 Compare
rock merged commit 6b18d81421 into main 2026-09-09 02:48:43 +00:00
rock deleted branch feat/phase-3.1-agent-prompt-entity 2026-09-09 02:49:04 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
memory-observability
Observability implementation tasks
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
poimen rock (Rock)
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: riotpiao-poimen/poimen-memory#47