.2869047221311500:dfcc5de8095b369d5063485b8b86dbbc_69e9cad9b964995ded0a5f13.69eb4d6daf036c7c109c4c2e.69eb4d6c71f280523c1e801d:Trae CN.T(2026/4/24 19:01:01)#98
Conversation
扩展Task模型和存储方法以支持团队协作功能,包括: - 添加任务所有者、审核者字段 - 实现任务状态管理 - 增加任务阻塞关系 - 添加事件标记和交接说明 - 实现截止日期和状态显示方法
- 将 `--task-status` 参数重命名为 `--status` 以保持一致性 - 为 `filter_apply` 命令添加团队协作筛选选项(负责人、评审人、状态等) - 改进任务列表展示,支持详细视图显示更多信息 - 扩展保存的过滤器功能,支持存储所有新筛选条件
There was a problem hiding this comment.
Pull request overview
This PR expands the task model and CLI to support team-collaboration workflows (ownership, review, statuses, due dates, blockers, incident flag), including richer filtering and reporting.
Changes:
- Added team-collaboration fields to
Taskplus helper methods for status/due/blocking logic. - Extended JSON storage
add_taskto persist the new fields. - Enhanced CLI
add/ls/edit/filterwith collaboration options/filters and addedhandoff+dailyreporting commands.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
tix/storage/json_storage.py |
Extends task creation to include collaboration fields in persisted JSON. |
tix/models.py |
Adds collaboration fields to Task, updates (de)serialization, and introduces helper methods (status, due dates, blockers). |
tix/cli.py |
Adds parsing helpers, new CLI options/filters, and new handoff/daily report commands. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Team collaboration fields with safe defaults | ||
| owner=data.get('owner'), | ||
| reviewer=data.get('reviewer'), | ||
| status=data.get('status', 'todo'), | ||
| due_at=data.get('due_at'), | ||
| handoff_note=data.get('handoff_note'), | ||
| blocked_by=data.get('blocked_by', []), | ||
| incident=data.get('incident', False), |
There was a problem hiding this comment.
Task.from_dict() defaults status to 'todo' even when the stored task is already completed=True (legacy tasks). This leads to inconsistent state (completed tasks showing non-done status) and affects status-based filtering/display. Consider defaulting status to 'done' when completed is true and no explicit status is present, and/or normalizing mismatched completed/status values during load.
| def needs_handoff(self) -> bool: | ||
| """Check if task needs handoff note (blocked but no note, or due soon without note)""" | ||
| if self.completed: | ||
| return False | ||
| # Blocked tasks should have handoff notes | ||
| if self.is_blocked() and not self.handoff_note: | ||
| return True | ||
| # Cross-person collaboration needs reviewer | ||
| if self.owner and not self.reviewer and self.status == 'review': | ||
| return True | ||
| return False |
There was a problem hiding this comment.
needs_handoff() docstring mentions "due soon without note", but the implementation only checks (a) blocked without note and (b) status=review without reviewer. Either add the due-soon rule (e.g., self.is_due_soon(...) and not self.handoff_note) or update the docstring/CLI semantics so the --no-handoff filter and 📋 icon match the actual criteria.
| status=status, | ||
| due_at=due_at, | ||
| handoff_note=handoff_note, | ||
| blocked_by=blocked_by or [], | ||
| incident=incident, | ||
| ) |
There was a problem hiding this comment.
TaskStorage.add_task() accepts a status value but passes it directly into the Task constructor. If a user creates a task with status='done' (allowed by the CLI choice list), the task will be stored with completed=False and no completed_at timestamp, which is inconsistent with Task.mark_done()/set_status(). Consider creating the Task with default status and then calling task.set_status(status) (or special-casing 'done' to call mark_done()) so completed/completed_at stay in sync.
| status=status, | |
| due_at=due_at, | |
| handoff_note=handoff_note, | |
| blocked_by=blocked_by or [], | |
| incident=incident, | |
| ) | |
| due_at=due_at, | |
| handoff_note=handoff_note, | |
| blocked_by=blocked_by or [], | |
| incident=incident, | |
| ) | |
| if status != 'todo': | |
| new_task.set_status(status) |
| lines.append(f"- [ ] **#{task.id}** {task.text}") | ||
| if task.due_at: | ||
| lines.append(f" - Due: {task.get_due_date_display()}") | ||
| lines.append("") |
There was a problem hiding this comment.
get_due_date_display() returns Rich markup (e.g., [red]OVERDUE...[/red]). In the markdown handoff report, those tags will render as literal text. Consider using a plain-string due date formatter for markdown output (or add a rich: bool/format= parameter) and reserve Rich markup for console/table rendering.
| if stats['overdue']: | ||
| lines.append("**Overdue:**") | ||
| for task in stats['overdue']: | ||
| lines.append(f"- [ ] #{task.id} {task.text} (Due: {task.get_due_date_display()})") |
There was a problem hiding this comment.
get_due_date_display() includes Rich markup, but the markdown daily report includes it inside plain markdown strings (e.g., the overdue list). This will leak [red]...[/red] tags into the markdown output. Use a non-Rich due-date string for markdown generation (or make get_due_date_display() configurable).
| lines.append(f"- [ ] #{task.id} {task.text} (Due: {task.get_due_date_display()})") | |
| due_date = getattr(task, "due_date", None) | |
| if due_date is None: | |
| due_date_display = "No due date" | |
| elif isinstance(due_date, datetime): | |
| due_date_display = due_date.strftime("%Y-%m-%d") | |
| elif hasattr(due_date, "strftime"): | |
| due_date_display = due_date.strftime("%Y-%m-%d") | |
| else: | |
| due_date_display = str(due_date) | |
| lines.append(f"- [ ] #{task.id} {task.text} (Due: {due_date_display})") |
| # Team collaboration helper methods | ||
| def is_blocked(self) -> bool: | ||
| """Check if task is blocked by other tasks""" | ||
| return len(self.blocked_by) > 0 | ||
|
|
||
| def set_blocked_by(self, task_ids: List[int]): | ||
| """Set which tasks block this one""" | ||
| self.blocked_by = task_ids | ||
| if task_ids: | ||
| self.status = 'blocked' | ||
|
|
||
| def add_blocker(self, task_id: int): | ||
| """Add a task that blocks this one""" | ||
| if task_id not in self.blocked_by and task_id != self.id: | ||
| self.blocked_by.append(task_id) | ||
| self.status = 'blocked' | ||
|
|
||
| def remove_blocker(self, task_id: int): | ||
| """Remove a blocking task""" | ||
| if task_id in self.blocked_by: | ||
| self.blocked_by.remove(task_id) | ||
| if not self.blocked_by and self.status == 'blocked': | ||
| self.status = 'todo' | ||
|
|
||
| def is_overdue(self) -> bool: | ||
| """Check if task is past its due date and not completed""" | ||
| if not self.due_at or self.completed: | ||
| return False | ||
| try: | ||
| due_date = datetime.fromisoformat(self.due_at).date() | ||
| today = datetime.now().date() | ||
| return due_date < today | ||
| except (ValueError, TypeError): | ||
| return False | ||
|
|
||
| def is_due_soon(self, days: int = 1) -> bool: | ||
| """Check if task is due within N days (default: 1 day)""" | ||
| if not self.due_at or self.completed: | ||
| return False | ||
| try: | ||
| due_date = datetime.fromisoformat(self.due_at).date() | ||
| today = datetime.now().date() | ||
| soon_date = today + timedelta(days=days) | ||
| return today <= due_date <= soon_date | ||
| except (ValueError, TypeError): | ||
| return False | ||
|
|
||
| def needs_handoff(self) -> bool: | ||
| """Check if task needs handoff note (blocked but no note, or due soon without note)""" | ||
| if self.completed: | ||
| return False | ||
| # Blocked tasks should have handoff notes | ||
| if self.is_blocked() and not self.handoff_note: | ||
| return True | ||
| # Cross-person collaboration needs reviewer | ||
| if self.owner and not self.reviewer and self.status == 'review': | ||
| return True | ||
| return False | ||
|
|
||
| def has_owner(self) -> bool: | ||
| """Check if task has an owner assigned""" | ||
| return self.owner is not None and self.owner.strip() != "" | ||
|
|
||
| def has_reviewer(self) -> bool: | ||
| """Check if task has a reviewer assigned""" | ||
| return self.reviewer is not None and self.reviewer.strip() != "" | ||
|
|
||
| def is_incident_task(self) -> bool: | ||
| """Check if this is an incident task""" | ||
| return self.incident | ||
|
|
||
| def set_status(self, new_status: str): | ||
| """Set task status with validation""" | ||
| if new_status not in STATUS_CHOICES: | ||
| raise ValueError(f"Invalid status. Must be one of: {STATUS_CHOICES}") | ||
| # If marking as done, also mark completed | ||
| if new_status == 'done' and not self.completed: | ||
| self.mark_done() | ||
| else: | ||
| self.status = new_status | ||
| # If reactivating a done task | ||
| if self.completed and new_status != 'done': | ||
| self.completed = False | ||
| self.completed_at = None | ||
|
|
||
| def get_status_color(self) -> str: | ||
| """Get Rich color for status display""" | ||
| status_colors = { | ||
| 'todo': 'dim', | ||
| 'in_progress': 'cyan', | ||
| 'review': 'yellow', | ||
| 'blocked': 'red', | ||
| 'done': 'green', | ||
| } | ||
| return status_colors.get(self.status, 'dim') | ||
|
|
||
| def get_due_date_display(self) -> str: | ||
| """Get formatted due date with status indicator""" | ||
| if not self.due_at: | ||
| return "-" | ||
| try: | ||
| due = datetime.fromisoformat(self.due_at) | ||
| due_str = due.strftime('%Y-%m-%d') | ||
| if self.is_overdue(): | ||
| return f"[red]OVERDUE: {due_str}[/red]" | ||
| elif self.is_due_soon(): | ||
| return f"[yellow]Soon: {due_str}[/yellow]" | ||
| return due_str | ||
| except (ValueError, TypeError): | ||
| return str(self.due_at) |
There was a problem hiding this comment.
The new team-collaboration behaviors in Task (blocking/unblocking, due date calculations, handoff heuristics, status validation) aren’t covered by the existing tests/test_models.py assertions (which currently only cover basic creation/serialization/mark_done). Adding tests for these helper methods (including status validation and overdue/due-soon logic) would make this change safer.
| from rich.prompt import Prompt | ||
| from rich.markdown import Markdown | ||
| from datetime import datetime | ||
| from typing import List |
There was a problem hiding this comment.
cli.py now imports List at the top (from typing import List) but later in the file there is already a from typing import Optional, Dict, Any. Consider consolidating these into a single import to avoid duplication and keep imports organized.
|
|
||
| # Apply due-soon filter | ||
| if due_soon: | ||
| tasks = [t for t in tasks if t.is_due_soon(days=2)] |
There was a problem hiding this comment.
The --due-soon filter help says "today/tomorrow", but the implementation uses t.is_due_soon(days=2), which includes tasks due up to two days out (inclusive). Either change days to 1 to match the help text, or update the option help to reflect the actual window.
| tasks = [t for t in tasks if t.is_due_soon(days=2)] | |
| tasks = [t for t in tasks if t.is_due_soon(days=1)] |
| def parse_due_date(date_str: str) -> str: | ||
| """Parse due date string into ISO format. | ||
| Supports: '2026-04-23', 'today', 'tomorrow', 'next-week', 'in-3-days' | ||
| """ | ||
| from datetime import datetime, timedelta | ||
|
|
||
| date_str = date_str.lower().strip() | ||
| today = datetime.now().date() | ||
|
|
||
| if date_str == 'today': | ||
| return datetime.combine(today, datetime.min.time()).isoformat() | ||
| elif date_str == 'tomorrow': | ||
| return datetime.combine(today + timedelta(days=1), datetime.min.time()).isoformat() | ||
| elif date_str == 'next-week' or date_str == 'nextweek': | ||
| return datetime.combine(today + timedelta(days=7), datetime.min.time()).isoformat() | ||
| elif date_str.startswith('in-') and date_str.endswith('-days'): | ||
| try: | ||
| days = int(date_str.split('-')[1]) | ||
| return datetime.combine(today + timedelta(days=days), datetime.min.time()).isoformat() | ||
| except (ValueError, IndexError): | ||
| pass | ||
| else: | ||
| # Try parsing as ISO date | ||
| try: | ||
| parsed = datetime.strptime(date_str, '%Y-%m-%d') | ||
| return parsed.isoformat() | ||
| except ValueError: | ||
| pass | ||
| try: | ||
| parsed = datetime.fromisoformat(date_str) | ||
| return parsed.isoformat() | ||
| except ValueError: | ||
| pass | ||
|
|
||
| raise ValueError(f"Invalid date format: {date_str}. Use YYYY-MM-DD, 'today', 'tomorrow', 'in-N-days'") | ||
|
|
||
|
|
||
| def parse_blocked_by(blocked_by_str: str) -> List[int]: | ||
| """Parse blocked-by string like '1,2,3' into list of integers""" | ||
| if not blocked_by_str: | ||
| return [] | ||
| parts = blocked_by_str.split(',') | ||
| result = [] | ||
| for p in parts: | ||
| p = p.strip() | ||
| if p: | ||
| try: | ||
| result.append(int(p)) | ||
| except ValueError: | ||
| raise ValueError(f"Invalid task ID: {p}") | ||
| return result |
There was a problem hiding this comment.
New parsing helpers (parse_due_date, parse_blocked_by) introduce non-trivial behavior (relative date keywords, error handling, parsing edge cases) but there don't appear to be unit tests covering them. Adding focused tests for accepted formats and invalid inputs will help prevent regressions, especially since these functions feed directly into persisted task data.
PR Checklist
What does this PR do?
Related Issue
Closes #
Type of change