forked from cardosofelipe/fast-next-template
fix(memory): unify Outcome enum and add ABANDONED support
- Add ABANDONED value to core Outcome enum in types.py - Replace duplicate OutcomeType class in mcp/tools.py with alias to Outcome - Simplify mcp/service.py to use outcome directly (no more silent mapping) - Add migration 0006 to extend PostgreSQL episode_outcome enum - Add missing constraints to migration 0005 (ix_facts_unique_triple_global) This fixes the semantic issue where ABANDONED outcomes were silently converted to FAILURE, losing information about task abandonment. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -300,6 +300,14 @@ def upgrade() -> None:
|
|||||||
unique=True,
|
unique=True,
|
||||||
postgresql_where=sa.text("project_id IS NOT NULL"),
|
postgresql_where=sa.text("project_id IS NOT NULL"),
|
||||||
)
|
)
|
||||||
|
# Unique constraint for global facts (project_id IS NULL)
|
||||||
|
op.create_index(
|
||||||
|
"ix_facts_unique_triple_global",
|
||||||
|
"facts",
|
||||||
|
["subject", "predicate", "object"],
|
||||||
|
unique=True,
|
||||||
|
postgresql_where=sa.text("project_id IS NULL"),
|
||||||
|
)
|
||||||
|
|
||||||
# =========================================================================
|
# =========================================================================
|
||||||
# Create procedures table
|
# Create procedures table
|
||||||
@@ -396,6 +404,11 @@ def upgrade() -> None:
|
|||||||
"facts",
|
"facts",
|
||||||
"confidence >= 0.0 AND confidence <= 1.0",
|
"confidence >= 0.0 AND confidence <= 1.0",
|
||||||
)
|
)
|
||||||
|
op.create_check_constraint(
|
||||||
|
"ck_facts_reinforcement_positive",
|
||||||
|
"facts",
|
||||||
|
"reinforcement_count >= 1",
|
||||||
|
)
|
||||||
|
|
||||||
# Procedure constraints
|
# Procedure constraints
|
||||||
op.create_check_constraint(
|
op.create_check_constraint(
|
||||||
@@ -476,11 +489,15 @@ def downgrade() -> None:
|
|||||||
# Drop check constraints first
|
# Drop check constraints first
|
||||||
op.drop_constraint("ck_procedures_failure_positive", "procedures", type_="check")
|
op.drop_constraint("ck_procedures_failure_positive", "procedures", type_="check")
|
||||||
op.drop_constraint("ck_procedures_success_positive", "procedures", type_="check")
|
op.drop_constraint("ck_procedures_success_positive", "procedures", type_="check")
|
||||||
|
op.drop_constraint("ck_facts_reinforcement_positive", "facts", type_="check")
|
||||||
op.drop_constraint("ck_facts_confidence_range", "facts", type_="check")
|
op.drop_constraint("ck_facts_confidence_range", "facts", type_="check")
|
||||||
op.drop_constraint("ck_episodes_tokens_positive", "episodes", type_="check")
|
op.drop_constraint("ck_episodes_tokens_positive", "episodes", type_="check")
|
||||||
op.drop_constraint("ck_episodes_duration_positive", "episodes", type_="check")
|
op.drop_constraint("ck_episodes_duration_positive", "episodes", type_="check")
|
||||||
op.drop_constraint("ck_episodes_importance_range", "episodes", type_="check")
|
op.drop_constraint("ck_episodes_importance_range", "episodes", type_="check")
|
||||||
|
|
||||||
|
# Drop unique indexes for global facts
|
||||||
|
op.drop_index("ix_facts_unique_triple_global", "facts")
|
||||||
|
|
||||||
# Drop tables in reverse order (dependencies first)
|
# Drop tables in reverse order (dependencies first)
|
||||||
op.drop_table("memory_consolidation_log")
|
op.drop_table("memory_consolidation_log")
|
||||||
op.drop_table("procedures")
|
op.drop_table("procedures")
|
||||||
|
|||||||
52
backend/app/alembic/versions/0006_add_abandoned_outcome.py
Normal file
52
backend/app/alembic/versions/0006_add_abandoned_outcome.py
Normal file
@@ -0,0 +1,52 @@
|
|||||||
|
"""Add ABANDONED to episode_outcome enum
|
||||||
|
|
||||||
|
Revision ID: 0006
|
||||||
|
Revises: 0005
|
||||||
|
Create Date: 2025-01-06
|
||||||
|
|
||||||
|
This migration adds the 'abandoned' value to the episode_outcome enum type.
|
||||||
|
This allows episodes to track when a task was abandoned (not completed,
|
||||||
|
but not necessarily a failure either - e.g., user cancelled, session timeout).
|
||||||
|
"""
|
||||||
|
|
||||||
|
from collections.abc import Sequence
|
||||||
|
|
||||||
|
from alembic import op
|
||||||
|
|
||||||
|
# revision identifiers, used by Alembic.
|
||||||
|
revision: str = "0006"
|
||||||
|
down_revision: str | None = "0005"
|
||||||
|
branch_labels: str | Sequence[str] | None = None
|
||||||
|
depends_on: str | Sequence[str] | None = None
|
||||||
|
|
||||||
|
|
||||||
|
def upgrade() -> None:
|
||||||
|
"""Add 'abandoned' value to episode_outcome enum."""
|
||||||
|
# PostgreSQL ALTER TYPE ADD VALUE is safe and non-blocking
|
||||||
|
op.execute("ALTER TYPE episode_outcome ADD VALUE IF NOT EXISTS 'abandoned'")
|
||||||
|
|
||||||
|
|
||||||
|
def downgrade() -> None:
|
||||||
|
"""Remove 'abandoned' from episode_outcome enum.
|
||||||
|
|
||||||
|
Note: PostgreSQL doesn't support removing values from enums directly.
|
||||||
|
This downgrade converts any 'abandoned' episodes to 'failure' and
|
||||||
|
recreates the enum without 'abandoned'.
|
||||||
|
"""
|
||||||
|
# Convert any abandoned episodes to failure first
|
||||||
|
op.execute("""
|
||||||
|
UPDATE episodes
|
||||||
|
SET outcome = 'failure'
|
||||||
|
WHERE outcome = 'abandoned'
|
||||||
|
""")
|
||||||
|
|
||||||
|
# Recreate the enum without abandoned
|
||||||
|
# This is complex in PostgreSQL - requires creating new type, updating columns, dropping old
|
||||||
|
op.execute("ALTER TYPE episode_outcome RENAME TO episode_outcome_old")
|
||||||
|
op.execute("CREATE TYPE episode_outcome AS ENUM ('success', 'failure', 'partial')")
|
||||||
|
op.execute("""
|
||||||
|
ALTER TABLE episodes
|
||||||
|
ALTER COLUMN outcome TYPE episode_outcome
|
||||||
|
USING outcome::text::episode_outcome
|
||||||
|
""")
|
||||||
|
op.execute("DROP TYPE episode_outcome_old")
|
||||||
@@ -1024,15 +1024,8 @@ class MemoryToolService:
|
|||||||
context: ToolContext,
|
context: ToolContext,
|
||||||
) -> dict[str, Any]:
|
) -> dict[str, Any]:
|
||||||
"""Execute the 'record_outcome' tool."""
|
"""Execute the 'record_outcome' tool."""
|
||||||
# Map outcome type to memory Outcome
|
# OutcomeType is now an alias for Outcome, use directly
|
||||||
# Note: ABANDONED maps to FAILURE since core Outcome doesn't have ABANDONED
|
outcome = args.outcome
|
||||||
outcome_map = {
|
|
||||||
OutcomeType.SUCCESS: Outcome.SUCCESS,
|
|
||||||
OutcomeType.PARTIAL: Outcome.PARTIAL,
|
|
||||||
OutcomeType.FAILURE: Outcome.FAILURE,
|
|
||||||
OutcomeType.ABANDONED: Outcome.FAILURE, # No ABANDONED in core enum
|
|
||||||
}
|
|
||||||
outcome = outcome_map.get(args.outcome, Outcome.FAILURE)
|
|
||||||
|
|
||||||
# Record in episodic memory
|
# Record in episodic memory
|
||||||
episodic = await self._get_episodic()
|
episodic = await self._get_episodic()
|
||||||
|
|||||||
@@ -12,6 +12,9 @@ from typing import Any
|
|||||||
|
|
||||||
from pydantic import BaseModel, Field
|
from pydantic import BaseModel, Field
|
||||||
|
|
||||||
|
# OutcomeType alias - uses core Outcome enum from types module for consistency
|
||||||
|
from app.services.memory.types import Outcome as OutcomeType
|
||||||
|
|
||||||
|
|
||||||
class MemoryType(str, Enum):
|
class MemoryType(str, Enum):
|
||||||
"""Types of memory for storage operations."""
|
"""Types of memory for storage operations."""
|
||||||
@@ -32,15 +35,6 @@ class AnalysisType(str, Enum):
|
|||||||
LEARNING_PROGRESS = "learning_progress"
|
LEARNING_PROGRESS = "learning_progress"
|
||||||
|
|
||||||
|
|
||||||
class OutcomeType(str, Enum):
|
|
||||||
"""Outcome types for record_outcome tool."""
|
|
||||||
|
|
||||||
SUCCESS = "success"
|
|
||||||
PARTIAL = "partial"
|
|
||||||
FAILURE = "failure"
|
|
||||||
ABANDONED = "abandoned"
|
|
||||||
|
|
||||||
|
|
||||||
# ============================================================================
|
# ============================================================================
|
||||||
# Tool Argument Schemas (Pydantic models for validation)
|
# Tool Argument Schemas (Pydantic models for validation)
|
||||||
# ============================================================================
|
# ============================================================================
|
||||||
|
|||||||
@@ -42,6 +42,7 @@ class Outcome(str, Enum):
|
|||||||
SUCCESS = "success"
|
SUCCESS = "success"
|
||||||
FAILURE = "failure"
|
FAILURE = "failure"
|
||||||
PARTIAL = "partial"
|
PARTIAL = "partial"
|
||||||
|
ABANDONED = "abandoned"
|
||||||
|
|
||||||
|
|
||||||
class ConsolidationStatus(str, Enum):
|
class ConsolidationStatus(str, Enum):
|
||||||
|
|||||||
Reference in New Issue
Block a user