StarPunk/docs/decisions/ADR-013-expose-deleted-at-in-note-model.md

ADR-013: Expose deleted_at Field in Note Model

Status

Accepted

Context

The StarPunk application implements soft deletion for notes, using a deleted_at timestamp in the database to mark notes as deleted without physically removing them. However, there is a model-schema mismatch: the deleted_at column exists in the database schema but is not exposed as a field in the Note dataclass.

Current State

Database Schema (starpunk/database.py):

CREATE TABLE IF NOT EXISTS notes (
    id INTEGER PRIMARY KEY AUTOINCREMENT,
    slug TEXT UNIQUE NOT NULL,
    file_path TEXT UNIQUE NOT NULL,
    published BOOLEAN DEFAULT 0,
    created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
    updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
    deleted_at TIMESTAMP,           -- Column exists
    content_hash TEXT
);

Note Model (starpunk/models.py):

@dataclass(frozen=True)
class Note:
    # Core fields from database
    id: int
    slug: str
    file_path: str
    published: bool
    created_at: datetime
    updated_at: datetime
    # deleted_at: MISSING

Notes Module (starpunk/notes.py):

• Uses deleted_at in queries (WHERE deleted_at IS NULL)
• Sets deleted_at during soft deletion (UPDATE notes SET deleted_at = ?)
• Never exposes the value through the model layer

Problem

This architecture creates several issues:

1. Testability Gap: Tests cannot verify soft-deletion status because note.deleted_at doesn't exist
2. Information Hiding: The model hides database state from consumers
3. Principle Violation: Data models should faithfully represent database schema
4. Future Limitations: Admin UIs, debugging tools, and backup utilities cannot access deletion timestamps

Immediate Trigger

Test test_delete_without_confirmation_cancels fails with:

AttributeError: 'Note' object has no attribute 'deleted_at'

The test attempts to verify that a cancelled deletion does NOT set deleted_at:

note = get_note(id=note_id)
assert note is not None
assert note.deleted_at is None  # ← Fails here

Decision

We will add deleted_at: Optional[datetime] as a field in the Note dataclass.

The field will be:

• Nullable: Optional[datetime] = None
• Extracted from database rows in Note.from_row()
• Documented in the Note docstring
• Optionally serialized in Note.to_dict() when present

Rationale

Why Add the Field

1. Transparency Over Encapsulation

   • For data models, transparency should win
   • Developers expect to access any database column through the model
   • Hiding fields creates semantic mismatches

2. Testability

   • Tests must be able to verify soft-deletion behavior
   • Current design makes deletion status verification impossible
   • Exposing the field enables proper test coverage

3. Principle of Least Surprise

   • If a database column exists, it should be accessible
   • Other models (Session, Token, AuthState) expose all their fields
   • Consistency across the codebase

4. Future Flexibility

   • Admin interfaces may need to show when notes were deleted
   • Data export/backup tools need complete state
   • Debugging requires visibility into deletion status

5. Low Complexity Cost

   • Adding one optional field is minimal complexity
   • No performance impact (no additional queries)
   • Backwards compatible (existing code won't break)

Why NOT Use Alternative Approaches

Alternative 1: Fix the Test Only

• Weakens test coverage (can't verify deletion status)
• Doesn't solve root problem (future code will hit same issue)
• Rejected

Alternative 2: Add Helper Property (is_deleted)

• Loses information (can't see deletion timestamp)
• Adds complexity (two fields instead of one)
• Inconsistent with other models
• Rejected

Alternative 3: Separate Model Class for Deleted Notes

• Massive complexity increase
• Violates simplicity principle
• Breaks existing code
• Rejected

Consequences

Positive Consequences

1. Test Suite Passes: test_delete_without_confirmation_cancels will pass
2. Complete Model: Note model accurately reflects database schema
3. Better Testability: All tests can verify soft-deletion state
4. Future-Proof: Admin UIs and debugging tools have access to deletion data
5. Consistency: All models expose their database fields

Negative Consequences

1. Loss of Encapsulation: Consumers now see deleted_at and must understand soft deletion

   • Mitigation: Document the field clearly in docstring
   • Impact: Minimal - developers working with notes should understand deletion

2. Slight Complexity Increase: Model has one more field

   • Impact: One line of code, negligible complexity

Breaking Changes

None - The field is optional and nullable, so:

• Existing code that doesn't use deleted_at continues to work
• Note.from_row() sets it to None for active notes
• Serialization is optional

Implementation Guidance

File: starpunk/models.py

Change 1: Add Field to Dataclass

@dataclass(frozen=True)
class Note:
    """Represents a note/post"""

    # Core fields from database
    id: int
    slug: str
    file_path: str
    published: bool
    created_at: datetime
    updated_at: datetime
    deleted_at: Optional[datetime] = None  # ← ADD THIS LINE

    # Internal fields (not from database)
    _data_dir: Path = field(repr=False, compare=False)

    # Optional fields
    content_hash: Optional[str] = None

Change 2: Update from_row() Method

Add timestamp conversion for deleted_at:

@classmethod
def from_row(cls, row: sqlite3.Row | dict[str, Any], data_dir: Path) -> "Note":
    # ... existing code ...

    # Convert timestamps if they are strings
    created_at = data["created_at"]
    if isinstance(created_at, str):
        created_at = datetime.fromisoformat(created_at.replace("Z", "+00:00"))

    updated_at = data["updated_at"]
    if isinstance(updated_at, str):
        updated_at = datetime.fromisoformat(updated_at.replace("Z", "+00:00"))

    # ← ADD THIS BLOCK
    deleted_at = data.get("deleted_at")
    if deleted_at and isinstance(deleted_at, str):
        deleted_at = datetime.fromisoformat(deleted_at.replace("Z", "+00:00"))

    return cls(
        id=data["id"],
        slug=data["slug"],
        file_path=data["file_path"],
        published=bool(data["published"]),
        created_at=created_at,
        updated_at=updated_at,
        deleted_at=deleted_at,  # ← ADD THIS LINE
        _data_dir=data_dir,
        content_hash=data.get("content_hash"),
    )

Change 3: Update Docstring

Add documentation for deleted_at:

@dataclass(frozen=True)
class Note:
    """
    Represents a note/post

    Attributes:
        id: Database ID (primary key)
        slug: URL-safe slug (unique)
        file_path: Path to markdown file (relative to data directory)
        published: Whether note is published (visible publicly)
        created_at: Creation timestamp (UTC)
        updated_at: Last update timestamp (UTC)
        deleted_at: Soft deletion timestamp (UTC, None if not deleted)  # ← ADD THIS LINE
        content_hash: SHA-256 hash of content (for integrity checking)
        # ... rest of docstring ...
    """

Change 4 (Optional): Update to_dict() Method

Add deleted_at to serialization when present:

def to_dict(
    self, include_content: bool = False, include_html: bool = False
) -> dict[str, Any]:
    data = {
        "id": self.id,
        "slug": self.slug,
        "title": self.title,
        "published": self.published,
        "created_at": self.created_at.strftime("%Y-%m-%dT%H:%M:%SZ"),
        "updated_at": self.updated_at.strftime("%Y-%m-%dT%H:%M:%SZ"),
        "permalink": self.permalink,
        "excerpt": self.excerpt,
    }

    # ← ADD THIS BLOCK (optional)
    if self.deleted_at is not None:
        data["deleted_at"] = self.deleted_at.strftime("%Y-%m-%dT%H:%M:%SZ")

    if include_content:
        data["content"] = self.content

    if include_html:
        data["html"] = self.html

    return data

Testing Strategy

Verification Steps

1. Run Failing Test:

   uv run pytest tests/test_routes_admin.py::TestDeleteRoute::test_delete_without_confirmation_cancels -v
   

   Should pass after changes.

2. Run Full Test Suite:

   uv run pytest
   

   Should pass with no regressions.

3. Manual Verification:

   # Active note should have deleted_at = None
   note = get_note(slug="active-note")
   assert note.deleted_at is None
   
   # Soft-deleted note should have deleted_at set
   delete_note(slug="test-note", soft=True)
   # Note: get_note() filters out soft-deleted notes
   # To verify, query database directly or use admin interface
   

Expected Test Coverage

• deleted_at is None for active notes
• deleted_at is None for newly created notes
• deleted_at is set after soft deletion (verify via database query)
• get_note() returns None for soft-deleted notes (existing behavior)
• list_notes() excludes soft-deleted notes (existing behavior)

Acceptance Criteria

• deleted_at field added to Note dataclass
• from_row() extracts and parses deleted_at from database rows
• from_row() handles deleted_at as ISO string
• from_row() handles deleted_at as None (active notes)
• Docstring updated to document deleted_at
• Test test_delete_without_confirmation_cancels passes
• Full test suite passes with no regressions
• Optional: to_dict() includes deleted_at when present

Alternatives Considered

1. Update Test to Remove deleted_at Check

Approach: Modify test to not verify deletion status

Pros:

• One line change
• Maintains current encapsulation

Cons:

• Weakens test coverage
• Doesn't solve root problem
• Violates test intent

Decision: Rejected - Band-aid solution

2. Add Helper Property Instead of Raw Field

Approach: Expose is_deleted boolean property, hide timestamp

Pros:

• Encapsulates implementation
• Simple boolean interface

Cons:

• Loses deletion timestamp information
• Inconsistent with other models
• More complex than exposing field directly

Decision: Rejected - Adds complexity without clear benefit

3. Create Separate SoftDeletedNote Model

Approach: Use different classes for active vs deleted notes

Pros:

• Type safety
• Clear separation

Cons:

• Massive complexity increase
• Violates simplicity principle
• Breaks existing code

Decision: Rejected - Over-engineered for V1

References

• Test Failure Analysis: /home/phil/Projects/starpunk/docs/reports/test-failure-analysis-deleted-at-attribute.md
• Database Schema: starpunk/database.py:11-27
• Note Model: starpunk/models.py:44-440
• Notes Module: starpunk/notes.py:685-849
• Failing Test: tests/test_routes_admin.py:435-441
• ADR-004: File-Based Note Storage (discusses soft deletion design)

Related Standards

• Data Model Design: Models should faithfully represent database schema
• Testability Principle: All business logic must be testable
• Principle of Least Surprise: Developers expect database columns to be accessible
• Transparency vs Encapsulation: For data models, transparency wins

---

Date: 2025-11-18 Author: StarPunk Architect Agent Status: Accepted