readyblockedneeds helpallepicskb

Add Role to CommentAuthor for agent comments

t-306·WorkTask·
·
·
·Omni/Task/Core.hs
Created1 month ago·Updated1 month ago

Description

Edit

Summary

Refactor CommentAuthor to support agent roles (Product, Designer, Engineer, Reviewer). This enables tracking which perspective an agent was using when it commented on a task.

Current State

data CommentAuthor = Human | Junior | System
  deriving (Show, Eq, Read, Generic)

data Comment = Comment
  { commentText :: Text,
    commentAuthor :: CommentAuthor,
    commentCreatedAt :: UTCTime
  }

Target State

data Role = Product | Designer | Engineer | Reviewer
  deriving (Show, Eq, Read, Generic)

data CommentAuthor 
  = Human 
  | Agent Role
  | System
  deriving (Show, Eq, Read, Generic)

-- Comment stays the same, just uses new CommentAuthor

CLI Changes

Update task comment to accept --role flag: 

task comment <id> <message> [--role=<role>] [--json]

# Examples:
task comment t-123 --role=product "Here's the spec..."   # Agent Product
task comment t-123 --role=engineer "Implementation notes" # Agent Engineer  
task comment t-123 "My thoughts"                          # Human (no role)

Valid roles: product, designer, engineer, reviewer

JSON Serialization

{"author": {"agent": "product"}, "text": "...", "created_at": "..."}
{"author": "human", "text": "...", "created_at": "..."}
{"author": "system", "text": "...", "created_at": "..."}

Migration

  • Junior becomes Agent Engineer (backward compatibility)
  • Existing comments with Junior author should be migrated or handled gracefully

Files to Modify

1. Omni/Task/Core.hs - Add Role type, update CommentAuthor 2. Omni/Task.hs - Update CLI docopt and move function to handle --role 3. Update JSON instances for new sum type structure 4. Database migration if comments are stored in SQLite (check schema)

Testing

1. Create comments with each role 2. Verify JSON serialization/deserialization roundtrips 3. Verify task show displays role correctly 4. Verify backward compat with existing Junior comments 5. Run bild --test Omni/Task.hs

Acceptance Criteria

  • [ ] Role type with Product, Designer, Engineer, Reviewer constructors
  • [ ] CommentAuthor refactored to Human | Agent Role | System
  • [ ] --role flag works in CLI
  • [ ] JSON serialization handles nested Agent structure
  • [ ] Existing comments still parse (backward compat)
  • [ ] task show displays role nicely (e.g., "[product]" prefix or similar)

Timeline (4)

🔄[human]Open → InProgress1 month ago
💬[human]1 month ago

CHANGES REQUESTED: ## Code Review for t-306: Add Role to CommentAuthor for agent comments

Task Requirements Check

1. **Role type with Product, Designer, Engineer, Reviewer constructors** ✅

  • Added data Role = ProductMgr | Designer | Engineer | Reviewer
  • Note: Used ProductMgr instead of Product to avoid conflict with Data.Semigroup.Product - good thinking

2. **CommentAuthor refactored to Human | Agent Role | System** ✅

  • Changed from Human | Junior | System to Human | Agent Role | System

3. **--role flag works in CLI** ✅

  • Added to docopt: task comment <id> <message> [--role=<role>] [--json]
  • Implemented parsing in move' function with valid roles: product, designer, engineer, reviewer

4. JSON serialization handles nested Agent structure ✅

  • Custom ToJSON/FromJSON instances implemented
  • Format: "human", "system", or {"agent": "product"}

5. Existing comments still parse (backward compat) ✅

  • JSON: "junior" and "Junior" parse to Agent Engineer
  • SQLite: "junior" maps to Agent Engineer

6. **task show displays role nicely** ✅

  • Added formatAuthor function displaying [product], [designer], etc.

Issues Found

1. Minor inconsistency in Role naming: The CLI uses --role=product but the Haskell type is ProductMgr. This is documented and intentional (avoiding name conflict), but the JSON serialization needs checking.

2. JSON serialization for Role: The Role type uses deriving Generic and instance ToJSON Role / instance FromJSON Role without customization. This means:

  • ProductMgr will serialize as "ProductMgr" not "product"
  • Let me check if this matches the spec...

The task spec says: json {"author": {"agent": "product"}, ...}

But with generic deriving, it will produce: json {"author": {"agent": "ProductMgr"}, ...}

This is a bug - the JSON output won't match the specified format.

3. SQLite field format: Uses "agent:product", "agent:engineer", etc. which is fine and handles backward compat with "junior" → Agent Engineer.

Build/Test Status

The build and test output shows the file was processed (* Omni/Task/Core.hs) but the output format is garbled with control characters. It appears to have completed without errors.

Lint failed due to lint: command not found which is an environment issue, not a code issue.

Critical Bug

The Role type needs custom JSON instances to serialize as lowercase strings ("product", "designer", etc.) instead of the constructor names ("ProductMgr", "Designer", etc.). The task spec is explicit about the JSON format: 

{"author": {"agent": "product"}, "text": "...", "created_at": "..."}

Currently, with generic deriving, it will produce "ProductMgr" instead of "product".

Fix needed: Add custom ToJSON and FromJSON instances for Role that map:

  • ProductMgr ↔ "product"
  • Designer ↔ "designer"
  • Engineer ↔ "engineer"
  • Reviewer ↔ "reviewer"
🔄[system]InProgress → Done1 month ago