Skip to content

feat: decouple task priority from task order#652

Merged
Wirasm merged 8 commits intomainfrom
feature/decouple-task-priority-from-order
Sep 17, 2025
Merged

feat: decouple task priority from task order#652
Wirasm merged 8 commits intomainfrom
feature/decouple-task-priority-from-order

Conversation

@leex279
Copy link
Copy Markdown
Collaborator

@leex279 leex279 commented Sep 13, 2025

Summary

Implements a dedicated priority system that operates independently from the existing task_order system, enabling users to set task priority without affecting visual drag-and-drop positioning.

Key Changes

  • Database: Added priority column with enum type and proper migration
  • Backend: Updated API and service layers to handle priority field
  • Frontend: Updated components and hooks for direct priority management
  • Cache: Fixed ETag invalidation for priority updates

User Experience

  • Users can change task priority via TaskCard dropdown or TaskEditModal
  • Drag-and-drop task reordering works independently of priority
  • Priority changes persist correctly after page refresh
  • All existing functionality continues working identically

Technical Implementation

  • Database migration with safe enum handling and data backfill
  • Backend validation with proper enum checking
  • Frontend decoupling from task_order conversion functions
  • Comprehensive cache invalidation for real-time updates

Test Plan

  • Database migration runs successfully
  • Priority changes via TaskCard dropdown persist correctly
  • Priority changes via TaskEditModal persist correctly
  • Drag-and-drop reordering doesn't affect priority
  • Priority survives page refreshes
  • All four priority levels work (critical, high, medium, low)
  • Cache invalidation works properly

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Add task priority (low/medium/high/critical) with server-backed updates; shown on Task Cards and in the Edit modal.
    • Enhance Task Cards with drag-and-drop reordering, edit/delete actions, hover highlighting, and multi-select.
  • Improvements

    • More reliable task list refresh after updates.
  • Migrations

    • Add priority column and index to tasks.
  • Tests

    • Update test data to include priority.
  • Documentation

    • Clarify that priority is database-backed and independent of task order.

This implements a dedicated priority system that operates independently
from the existing task_order system, allowing users to set task priority
without affecting visual drag-and-drop positioning.

## Changes Made

### Database
- Add priority column to archon_tasks table with enum type (critical, high, medium, low)
- Create database migration with safe enum handling and data backfill
- Add proper indexing for performance

### Backend
- Update UpdateTaskRequest to include priority field
- Add priority validation in TaskService with enum checking
- Include priority field in task list responses and ETag generation
- Fix cache invalidation for priority updates

### Frontend
- Update TaskPriority type from "urgent" to "critical" for consistency
- Add changePriority method to useTaskActions hook
- Update TaskCard to use direct priority field instead of task_order conversion
- Update TaskEditModal priority form to use direct priority values
- Fix TaskPriorityComponent to use correct priority enum values
- Update buildTaskUpdates to include priority field changes
- Add priority field to Task interface as required field
- Update test fixtures to include priority field

## Key Features
- ✅ Users can change task priority without affecting drag-and-drop order
- ✅ Users can drag tasks to reorder without changing priority level
- ✅ Priority persists correctly in database with dedicated column
- ✅ All existing priority functionality continues working identically
- ✅ Cache invalidation works properly for priority changes
- ✅ Both TaskCard priority button and TaskEditModal priority work

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Sep 13, 2025

Walkthrough

Implements server-backed task priority across UI, hooks, API, services, and database. Replaces local priority state with persisted TaskPriority, updates types/interfaces, adds changePriority action, adjusts tests, extends TaskCard props for DnD/edit/select, updates cache invalidation, and adds priority enum/column with migrations.

Changes

Cohort / File(s) Summary
Task UI components
archon-ui-main/src/features/projects/tasks/components/TaskCard.tsx, .../components/TaskPriorityComponent.tsx, .../components/TaskEditModal.tsx, .../components/index.ts
Connect priority UI to backend; replace local state with TaskPriorityComponent using task.priority; add drag/reorder/edit/select props to TaskCard; update types to shared TaskPriority; rename/export TaskPriorityComponent.
Task hooks
archon-ui-main/src/features/projects/tasks/hooks/useTaskActions.ts, .../hooks/useTaskEditor.ts, .../hooks/tests/useTaskQueries.test.ts
Add changePriority(taskId, newPriority) action; include priority in create/update payloads; update tests to include priority: "medium".
Task types
archon-ui-main/src/features/projects/tasks/types/task.ts, .../types/priority.ts, .../types/hooks.ts
Make Task.priority required; switch priority option values to string TaskPriority; add isValidTaskPriority; update helper signatures; expose changePriority in hook typings.
Frontend services
archon-ui-main/src/features/projects/tasks/services/taskService.ts
Invalidate per-project task cache on update using task.project_id, in addition to counts cache.
API routes (server)
python/src/server/api_routes/projects_api.py
Extend create/update request models to include priority; include priority in ETAG snapshot; pass priority through to service.
Server services
python/src/server/services/projects/task_service.py
Validate priority; persist priority on create/update; include priority in list responses; default to "medium" when applicable.
Migrations
migration/add_priority_column_to_tasks.sql, migration/complete_setup.sql
Add enum task_priority and archon_tasks.priority column with default 'medium'; create index and comments; safe/idempotent initialization and constraints.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant TC as TaskCard (UI)
  participant H as useTaskActions
  participant Q as updateTaskMutation (Query)
  participant API as PATCH /projects/:id/tasks/:taskId
  participant S as TaskService.update_task
  participant DB as DB

  U->>TC: Change priority (select)
  TC->>H: changePriority(taskId, newPriority)
  H->>Q: mutate({ taskId, updates:{ priority } })
  Q->>API: Request with { priority }
  API->>S: update_task(update_fields["priority"])
  S->>DB: UPDATE archon_tasks SET priority=...
  DB-->>S: OK
  S-->>API: Updated task (priority)
  API-->>Q: 200 OK
  Q->>TC: Invalidate & refetch project tasks
  TC-->>U: UI reflects new priority (loading → updated)
Loading
sequenceDiagram
  autonumber
  participant U as User
  participant TM as TaskEditModal (UI)
  participant HE as useTaskEditor
  participant API as POST /projects/:id/tasks
  participant S as TaskService.create_task
  participant DB as DB

  U->>TM: Create task (select priority)
  TM->>HE: submit({ title, ..., priority })
  HE->>API: create_task(..., priority || "medium")
  API->>S: validate_priority + persist
  S->>DB: INSERT task with priority
  DB-->>S: OK
  S-->>API: Created task (priority)
  API-->>TM: 201 Created
  TM-->>U: Shows task with set priority
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • coleam00

Poem

I nudge a card with twitchy glee,
From low to high—now saved in DB.
Enum skies and indexed trails,
Hop through hooks and backend sails.
Cache burrows fresh and bright,
Medium by default—just right.
Carrot-approved: priorities in sight! 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title "feat: decouple task priority from task order" is concise, accurately summarizes the primary change, and directly reflects the PR's main intent to separate priority from visual ordering across database, backend, and frontend layers.
Description Check ✅ Passed The PR description contains a clear Summary, Key Changes, Technical Implementation notes, and a detailed Test Plan that together cover the substantive work, but it does not strictly follow the repository's required template: it lacks a dedicated "Changes Made" bullet list, the "Type of Change" and "Affected Services" checkbox sections, an explicit "Test Evidence" code block, and the standard Checklist and "Breaking Changes" section.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/decouple-task-priority-from-order

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

- Add task_priority enum type (low, medium, high, critical)
- Add priority column to archon_tasks table with default 'medium'
- Add index for priority column performance
- Add documentation comment for priority field

This ensures fresh installations include the priority system without
needing to run the separate migration.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@leex279
Copy link
Copy Markdown
Collaborator Author

leex279 commented Sep 13, 2025

@CodeRabbit review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Sep 13, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
archon-ui-main/src/features/projects/tasks/services/taskService.ts (1)

114-123: Also invalidate the project task list on status change.

Status updates change which column a task appears in; without invalidating the project list, UI may show stale data.

       const task = await callAPIWithETag<Task>(`/api/tasks/${taskId}`, {
         method: "PUT",
         body: JSON.stringify({ status }),
       });

-      // Invalidate task counts cache when status changes
-      invalidateETagCache("/api/tasks/counts");
+      // Invalidate caches
+      invalidateETagCache("/api/tasks/counts");
+      if (task.project_id) {
+        invalidateETagCache(`/api/projects/${task.project_id}/tasks`);
+      }
python/src/server/services/projects/task_service.py (1)

324-383: Create should accept and return priority

Parity with update/list is missing: create_task ignores priority and does not return it. This breaks UI expectations and the new required Task shape.

Suggested changes (outside the shown hunk):

-    async def create_task(
+    async def create_task(
         self,
         project_id: str,
         title: str,
         description: str = "",
         assignee: str = "User",
         task_order: int = 0,
         feature: str | None = None,
+        priority: str | None = None,
         sources: list[dict[str, Any]] = None,
         code_examples: list[dict[str, Any]] = None,
     ) -> tuple[bool, dict[str, Any]]:
@@
-            task_data = {
+            task_data = {
                 "project_id": project_id,
                 "title": title,
                 "description": description,
                 "status": task_status,
                 "assignee": assignee,
                 "task_order": task_order,
                 "sources": sources or [],
                 "code_examples": code_examples or [],
                 "created_at": datetime.now().isoformat(),
                 "updated_at": datetime.now().isoformat(),
             }
 
             if feature:
                 task_data["feature"] = feature
+
+            # Optional priority on create (validated); omit to use DB default
+            if priority is not None:
+                is_valid, error_msg = self.validate_priority(priority)
+                if not is_valid:
+                    return False, {"error": error_msg}
+                task_data["priority"] = priority
@@
-                return True, {
+                return True, {
                     "task": {
                         "id": task["id"],
                         "project_id": task["project_id"],
                         "title": task["title"],
                         "description": task["description"],
                         "status": task["status"],
                         "assignee": task["assignee"],
                         "task_order": task["task_order"],
+                        "priority": task.get("priority", "medium"),
                         "created_at": task["created_at"],
                     }
                 }
🧹 Nitpick comments (22)
migration/complete_setup.sql (1)

644-644: Consider composite index for common filters.

If the UI frequently queries tasks by project_id + priority, add a composite index to cut scans.

Apply:

CREATE INDEX IF NOT EXISTS idx_archon_tasks_project_priority ON archon_tasks(project_id, priority);
migration/add_priority_column_to_tasks.sql (1)

30-45: Backfill is fine; minor robustness nit.

Conditioning on priority='medium' is correct post-default. If any NULLs existed pre-default, extend predicate.

-WHERE priority = 'medium';  -- Only update records with default priority
+WHERE priority IS NULL OR priority = 'medium';  -- Only update records with default/defaulted priority
archon-ui-main/src/features/projects/tasks/services/taskService.ts (1)

88-91: Good: project-scoped cache invalidation after update.

Using returned task.project_id avoids stale lists. Consider also falling back to updates.project_id if server omits it.

-      if (task.project_id) {
+      if (task.project_id) {
         invalidateETagCache(`/api/projects/${task.project_id}/tasks`);
-      }
+      } else if ((validation.data as any)?.project_id) {
+        invalidateETagCache(`/api/projects/${(validation.data as any).project_id}/tasks`);
+      }
archon-ui-main/src/features/projects/tasks/types/priority.ts (2)

16-21: Centralized options look good; export order helper for sorting.

Many UIs sort by priority; provide a stable rank map to avoid ad-hoc comparisons.

 export const TASK_PRIORITY_OPTIONS: readonly TaskPriorityOption[] = [
   { value: "critical", label: "Critical", color: "text-red-600" },
   { value: "high", label: "High", color: "text-orange-600" },
   { value: "medium", label: "Medium", color: "text-blue-600" },
   { value: "low", label: "Low", color: "text-gray-600" },
 ] as const;
+
+export const TASK_PRIORITY_RANK: Readonly<Record<TaskPriority, number>> = {
+  critical: 0,
+  high: 1,
+  medium: 2,
+  low: 3,
+} as const;

31-36: Avoid duplicating allowed values — derive from options.

Prevents drift if enum expands.

-export function isValidTaskPriority(priority: string): priority is TaskPriority {
-  return ["critical", "high", "medium", "low"].includes(priority);
-}
+const PRIORITY_VALUES = TASK_PRIORITY_OPTIONS.map(o => o.value) as readonly TaskPriority[];
+export function isValidTaskPriority(priority: string): priority is TaskPriority {
+  return (PRIORITY_VALUES as readonly string[]).includes(priority);
+}
archon-ui-main/src/features/projects/tasks/components/TaskPriority.tsx (6)

4-8: Update header comment: backend now supports priority.

Text still says “frontend-only”. Align with the new server-backed field.

- * Display-only priority selector for tasks.
- * NOTE: Priority is currently frontend-only and doesn't affect task ordering.
- * Task ordering is handled separately via drag-and-drop with task_order field.
- * This is purely for visual categorization until backend priority support is added.
+ * Priority selector for tasks.
+ * Priority is server-backed and independent of drag-and-drop task_order.
+ * Task ordering uses task_order; semantic importance uses priority.

23-32: De-duplicate PRIORITY_OPTIONS — reuse the shared constant.

Keeps labels/colors/types in one place.

-// Priority options for the dropdown
-const PRIORITY_OPTIONS: Array<{
-  value: TaskPriority;
-  label: string;
-  color: string;
-}> = [
-  { value: "critical", label: "Critical", color: "text-red-600" },
-  { value: "high", label: "High", color: "text-orange-600" },
-  { value: "medium", label: "Medium", color: "text-blue-600" },
-  { value: "low", label: "Low", color: "text-gray-600" },
-];
+// Reuse shared TASK_PRIORITY_OPTIONS from the types module

77-79: Use shared lookup for current option.

Avoid hardcoded fallback index.

-  const currentOption = PRIORITY_OPTIONS.find((opt) => opt.value === priority) || PRIORITY_OPTIONS[2]; // Default to medium
+  const currentOption = getTaskPriorityOption(priority);

102-104: Validate select value before propagating.

Prevents accidental invalid values from custom Select.

-    <Select value={priority} onValueChange={(value) => onPriorityChange(value as TaskPriority)}>
+    <Select
+      value={priority}
+      onValueChange={(value) => {
+        if (isValidTaskPriority(value)) onPriorityChange(value);
+      }}
+    >

126-139: Use shared options for items.

Keeps UI in sync with enum.

-        {PRIORITY_OPTIONS.map((option) => {
+        {TASK_PRIORITY_OPTIONS.map((option) => {

14-14: Import shared priority helpers from types/priority (or re-export them from the barrel)

priority.ts exports TASK_PRIORITY_OPTIONS, getTaskPriorityOption and isValidTaskPriority; types/task.ts only re-exports TaskPriority — import the helpers from "../types/priority" or add those re-exports to the barrel.

-import type { TaskPriority } from "../types";
+import { TASK_PRIORITY_OPTIONS, getTaskPriorityOption, isValidTaskPriority, type TaskPriority } from "../types/priority";
archon-ui-main/src/features/projects/tasks/hooks/useTaskEditor.ts (3)

26-29: Nit: clarify comment (this is order, not priority)

Rename to avoid confusion with the new priority field.

-    // Simple priority mapping: todo=50, doing=25, review=75, done=100
+    // Default task_order mapping: todo=50, doing=25, review=75, done=100

35-41: Safer defaults with nullish coalescing

Avoid overwriting intentional falsy values using ||. Prefer ?? for assignee and feature.

-    if (localTask.assignee !== editingTask.assignee) updates.assignee = localTask.assignee || "User";
+    if (localTask.assignee !== editingTask.assignee) updates.assignee = localTask.assignee ?? "User";-    if (localTask.feature !== editingTask.feature) updates.feature = localTask.feature || "";
+    if (localTask.feature !== editingTask.feature) updates.feature = localTask.feature ?? "";

76-96: Skip network call when nothing changed

Avoid touching updated_at and cache churn for no-op updates.

       if (editingTask?.id) {
         // Update existing task
         const updates = buildTaskUpdates(localTask, editingTask);

+        if (Object.keys(updates).length === 0) {
+          onSuccess?.();
+          return;
+        }
archon-ui-main/src/features/projects/tasks/hooks/tests/useTaskQueries.test.ts (1)

72-73: Add a test for creating with explicit priority

Current tests only cover defaulted priority. Add a case where the create payload includes priority: "high" and assert it is forwarded to the service.

Example:

it("should send priority when provided on create", async () => {
  const created: Task = {
    id: "id-1",
    project_id: "project-123",
    title: "With Priority",
    description: "",
    status: "todo",
    assignee: "User",
    task_order: 100,
    priority: "high",
    created_at: "2024-01-01T00:00:00Z",
    updated_at: "2024-01-01T00:00:00Z",
  };
  const { taskService } = await import("../../services");
  vi.mocked(taskService.createTask).mockResolvedValue(created);

  const wrapper = createWrapper();
  const { result } = renderHook(() => useCreateTask(), { wrapper });

  await result.current.mutateAsync({
    project_id: "project-123",
    title: "With Priority",
    description: "",
    status: "todo",
    assignee: "User",
    priority: "high",
  });

  await waitFor(() => {
    expect(taskService.createTask).toHaveBeenCalledWith(
      expect.objectContaining({ priority: "high" })
    );
  });
});

Also applies to: 124-125, 164-165

archon-ui-main/src/features/projects/tasks/components/TaskEditModal.tsx (1)

21-22: Align modal create flow with backend support

UI captures priority for new tasks, but until the create endpoint accepts it, the selection won’t persist. After backend support (see API/Service comments), this modal will work end-to-end. No UI changes required beyond ensuring saveTask includes priority (addressed in useTaskEditor).

Minor: the cast "medium" as TaskPriority is redundant once priority is typed as Task["priority"]; consider a typed constant DEFAULT_PRIORITY: TaskPriority = "medium" for reuse.

Also applies to: 54-55, 135-138

python/src/server/services/projects/task_service.py (2)

45-54: LGTM: priority validation added

Allowed set matches enum. Consider promoting VALID_PRIORITIES to a class constant to mirror VALID_STATUSES.

-    def validate_priority(self, priority: str) -> tuple[bool, str]:
-        """Validate task priority against allowed enum values"""
-        VALID_PRIORITIES = ["low", "medium", "high", "critical"]
+    VALID_PRIORITIES = ["low", "medium", "high", "critical"]
+
+    def validate_priority(self, priority: str) -> tuple[bool, str]:
+        """Validate task priority against allowed enum values"""
-        if priority not in VALID_PRIORITIES:
+        if priority not in self.VALID_PRIORITIES:
             return (
                 False,
-                f"Invalid priority '{priority}'. Must be one of: {', '.join(VALID_PRIORITIES)}",
+                f"Invalid priority '{priority}'. Must be one of: {', '.join(self.VALID_PRIORITIES)}",
             )
         return True, ""

176-181: Optional: honor exclude_large_fields fully

You still select sources, code_examples when exclude_large_fields is true, negating payload savings. Consider omitting these columns and deriving counts separately if needed.

archon-ui-main/src/features/projects/tasks/hooks/useTaskActions.ts (1)

24-33: Add a no-op guard or optimistic update for smoother UX.

As-is, this will POST even if the user selects the already-active priority. Either:

  • Guard at the call site (preferred, since the hook doesn’t have current value), or
  • Add an optimistic update in the mutation layer so UI flips immediately.

See suggestion on TaskCard for the guard.

archon-ui-main/src/features/projects/tasks/components/TaskCard.tsx (3)

10-10: Consider importing directly from the module to avoid barrel cycles.

from "." can create subtle circular deps if this file is also re-exported. Prefer from "./TaskPriority" unless your barrel is known-acyclic.


58-63: Avoid redundant mutations when value doesn’t change.

Guard against sending an update if the chosen priority equals the current one.

Apply:

-  const handlePriorityChange = useCallback(
-    (priority: TaskPriority) => {
-      changePriority(task.id, priority);
-    },
-    [changePriority, task.id],
-  );
+  const handlePriorityChange = useCallback(
+    (priority: TaskPriority) => {
+      if (priority === task.priority) return;
+      changePriority(task.id, priority);
+    },
+    [changePriority, task.id, task.priority],
+  );

219-224: Show per-card loading state to prevent global spinners.

isUpdating likely reflects any task update, so every card may show loading during unrelated mutations. Expose an updatingTaskId from the hook or a predicate (e.g., isUpdatingTask(task.id)) and pass that here.

I can wire this by extending useTaskActions to track the last taskId mutated (via onMutate) and return updatingTaskId; then replace isLoading={isUpdating} with isLoading={isUpdating && updatingTaskId === task.id}.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ce2f871 and 63d9a13.

📒 Files selected for processing (15)
  • archon-ui-main/src/features/projects/tasks/components/TaskCard.tsx (4 hunks)
  • archon-ui-main/src/features/projects/tasks/components/TaskEditModal.tsx (3 hunks)
  • archon-ui-main/src/features/projects/tasks/components/TaskPriority.tsx (3 hunks)
  • archon-ui-main/src/features/projects/tasks/components/index.ts (1 hunks)
  • archon-ui-main/src/features/projects/tasks/hooks/tests/useTaskQueries.test.ts (3 hunks)
  • archon-ui-main/src/features/projects/tasks/hooks/useTaskActions.ts (3 hunks)
  • archon-ui-main/src/features/projects/tasks/hooks/useTaskEditor.ts (1 hunks)
  • archon-ui-main/src/features/projects/tasks/services/taskService.ts (1 hunks)
  • archon-ui-main/src/features/projects/tasks/types/hooks.ts (1 hunks)
  • archon-ui-main/src/features/projects/tasks/types/priority.ts (1 hunks)
  • archon-ui-main/src/features/projects/tasks/types/task.ts (1 hunks)
  • migration/add_priority_column_to_tasks.sql (1 hunks)
  • migration/complete_setup.sql (4 hunks)
  • python/src/server/api_routes/projects_api.py (3 hunks)
  • python/src/server/services/projects/task_service.py (4 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-06T20:04:08.138Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T20:04:08.138Z
Learning: Applies to archon-ui-main/src/features/**/tasks/**/*.{ts,tsx} : Use task status values directly from the database: todo, doing, review, done (no UI mapping)

Applied to files:

  • archon-ui-main/src/features/projects/tasks/hooks/tests/useTaskQueries.test.ts
  • archon-ui-main/src/features/projects/tasks/types/hooks.ts
  • archon-ui-main/src/features/projects/tasks/services/taskService.ts
  • archon-ui-main/src/features/projects/tasks/hooks/useTaskEditor.ts
  • archon-ui-main/src/features/projects/tasks/components/TaskEditModal.tsx
  • archon-ui-main/src/features/projects/tasks/components/index.ts
  • archon-ui-main/src/features/projects/tasks/types/task.ts
  • archon-ui-main/src/features/projects/tasks/hooks/useTaskActions.ts
  • archon-ui-main/src/features/projects/tasks/types/priority.ts
  • archon-ui-main/src/features/projects/tasks/components/TaskPriority.tsx
  • archon-ui-main/src/features/projects/tasks/components/TaskCard.tsx
📚 Learning: 2025-09-12T13:47:04.534Z
Learnt from: CR
PR: coleam00/Archon#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:47:04.534Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Follow TanStack Query patterns: query-keys factory, smart polling via useSmartPolling, optimistic updates with rollback

Applied to files:

  • archon-ui-main/src/features/projects/tasks/services/taskService.ts
📚 Learning: 2025-09-06T20:04:08.138Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T20:04:08.138Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Implement optimistic updates with rollback in TanStack Query mutations

Applied to files:

  • archon-ui-main/src/features/projects/tasks/services/taskService.ts
📚 Learning: 2025-08-28T12:56:47.840Z
Learnt from: Wirasm
PR: coleam00/Archon#514
File: archon-ui-main/src/pages/ProjectPage.tsx:329-331
Timestamp: 2025-08-28T12:56:47.840Z
Learning: In the ProjectPage.tsx polling refactor, temporary project creation logic with progress cards was removed in favor of simpler modal loading states, as the complexity of managing in-flight temporary projects wasn't justified when HTTP polling would show new projects within seconds anyway.

Applied to files:

  • archon-ui-main/src/features/projects/tasks/components/TaskCard.tsx
🔇 Additional comments (16)
migration/complete_setup.sql (2)

560-566: Enum creation is safe and idempotent — good choice.

Catching duplicate_object avoids failures on re-runs. LGTM.


707-707: Comment aligns with enum values — good.

Matches low/medium/high/critical and clarifies decoupling from order.

archon-ui-main/src/features/projects/tasks/services/taskService.ts (1)

134-142: Do not change deleteTask signature — cache invalidation is already handled.

useDeleteTask(projectId) is the only caller (archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts) and it optimistically removes the task from taskKeys.all(projectId) and invalidates counts onSettled, so adding a projectId param to taskService.deleteTask is unnecessary.

Likely an incorrect or invalid review comment.

archon-ui-main/src/features/projects/tasks/types/priority.ts (1)

4-6: Clear separation from task_order is good.

The doc explains the decoupling intent succinctly.

archon-ui-main/src/features/projects/tasks/components/TaskPriority.tsx (1)

34-41: Component rename and typing look good.

Prop types now use TaskPriority; internal helpers typed accordingly. LGTM.

archon-ui-main/src/features/projects/tasks/types/task.ts (1)

62-64: LGTM: priority required on Task surface

Matches DB-backed model; create/update requests keep priority optional for flexibility. Ensure server returns priority on create (see backend comments).

archon-ui-main/src/features/projects/tasks/types/hooks.ts (1)

7-15: LGTM: expose changePriority in hook typings

API shape matches usage in components and actions.

python/src/server/api_routes/projects_api.py (2)

582-590: LGTM: include priority in ETag snapshot

Ensures priority changes invalidate caches correctly.


767-775: LGTM: UpdateTaskRequest adds priority

Interfaces with service validation; good.

archon-ui-main/src/features/projects/tasks/components/index.ts (1)

16-16: LGTM: direct export is clearer

Removes alias indirection; consistent with renamed component.

python/src/server/services/projects/task_service.py (2)

356-361: LGTM: update path validates and persists priority

Matches API additions and ETag handling.


263-264: Good defaulting in listing

Including priority with a sane default avoids breaking older rows during rollout.

archon-ui-main/src/features/projects/tasks/hooks/useTaskActions.ts (2)

2-2: Type import looks correct.

Good to see TaskPriority pulled from shared types to enforce the enum at the edges.


68-68: Public hook type includes changePriority — resolved.
UseTaskActionsReturn declares changePriority (archon-ui-main/src/features/projects/tasks/types/hooks.ts:15) and call sites compile (archon-ui-main/src/features/projects/tasks/components/TaskCard.tsx:60).

archon-ui-main/src/features/projects/tasks/components/TaskCard.tsx (2)

6-6: Type import alignment is good.

Using TaskPriority here keeps UI in lockstep with backend enum.


37-39: Hook usage is clean.

Plumbing changePriority via useTaskActions(projectId) keeps responsibilities separated.

Comment thread archon-ui-main/src/features/projects/tasks/hooks/useTaskEditor.ts
Comment thread migration/add_priority_column_to_tasks.sql
Comment thread migration/add_priority_column_to_tasks.sql Outdated
Comment thread migration/add_priority_column_to_tasks.sql Outdated
Comment thread migration/add_priority_column_to_tasks.sql Outdated
Comment thread migration/complete_setup.sql Outdated
Comment thread python/src/server/api_routes/projects_api.py
leex279 and others added 4 commits September 13, 2025 13:31
When creating new tasks via TaskEditModal, the buildCreateRequest function
was not including the priority field, causing new tasks to fall back to
the database default ('medium') instead of respecting the user's selected
priority in the modal.

Added priority: localTask.priority || 'medium' to ensure the user's
chosen priority is sent to the API during task creation.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Replaced destructive DROP TYPE CASCADE with safe migration patterns:

- Use DO blocks with EXCEPTION handling for enum and column creation
- Prevent conflicts with complete_setup.sql for fresh installations
- Enhanced backfill logic to preserve user-modified priorities
- Only update tasks that haven't been modified (updated_at = created_at)
- Add comprehensive error handling with informative notices
- Migration can now be run multiple times safely

This ensures the migration works for both existing installations
(incremental migration) and fresh installations (complete_setup.sql)
without data loss or conflicts.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Data integrity improvements:

Migration (add_priority_column_to_tasks.sql):
- Add column as nullable first with DEFAULT 'medium'
- Update any NULL values to 'medium'
- Set NOT NULL constraint to enforce application invariants
- Safe handling for existing columns with proper constraint checking

Complete Setup (complete_setup.sql):
- Priority column now DEFAULT 'medium' NOT NULL for fresh installations
- Ensures consistency between migration and fresh install paths

Both paths now enforce priority field as required, matching the
frontend Task interface where priority is a required field.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Complete priority support for task creation:

API Routes (projects_api.py):
- Add priority field to CreateTaskRequest Pydantic model
- Pass request.priority to TaskService.create_task call

Task Service (task_service.py):
- Add priority parameter to create_task method signature
- Add priority validation using existing validate_priority method
- Include priority field in database INSERT task_data
- Include priority field in API response task object

This ensures that new tasks created via TaskEditModal respect the
user's selected priority instead of falling back to database default.

Validation ensures only valid priority values (low, medium, high, critical)
are accepted and stored in the database.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@leex279 leex279 marked this pull request as ready for review September 13, 2025 11:43
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (6)
python/src/server/services/projects/task_service.py (3)

45-54: Hoist and normalize priority validation

  • Move allowed values to a class-level constant to avoid per-call allocation and ensure single source of truth.
  • Consider lowercasing input before validation to be resilient to UI input casing.

Apply within this block:

-    def validate_priority(self, priority: str) -> tuple[bool, str]:
-        """Validate task priority against allowed enum values"""
-        VALID_PRIORITIES = ["low", "medium", "high", "critical"]
-        if priority not in VALID_PRIORITIES:
+    def validate_priority(self, priority: str) -> tuple[bool, str]:
+        """Validate task priority against allowed enum values"""
+        priority = priority.lower().strip()
+        if priority not in self.VALID_PRIORITIES:
             return (
                 False,
-                f"Invalid priority '{priority}'. Must be one of: {', '.join(VALID_PRIORITIES)}",
+                f"Invalid priority '{priority}'. Must be one of: {', '.join(self.VALID_PRIORITIES)}",
             )
         return True, ""

Add outside this range (near VALID_STATUSES):

 class TaskService:
     """Service class for task operations"""
 
     VALID_STATUSES = ["todo", "doing", "review", "done"]
+    VALID_PRIORITIES = ["low", "medium", "high", "critical"]

62-63: Type the priority parameter for stronger contracts

Using a Literal improves MyPy guarantees and prevents accidental values flowing in.

Apply outside this range (imports):

-from typing import Any
+from typing import Any, Literal

And here:

-        priority: str = "medium",
+        priority: Literal["low", "medium", "high", "critical"] = "medium",

364-369: LGTM: guarded priority update

Validation before update is correct. Consider lowercasing here too for consistency with validate_priority normalization.

-                update_data["priority"] = update_fields["priority"]
+                update_data["priority"] = update_fields["priority"].lower().strip()
migration/add_priority_column_to_tasks.sql (3)

17-25: Ensure enum has expected labels when type already exists

If task_priority exists but is missing some labels, subsequent inserts can fail. Add idempotent ADD VALUE guards in the duplicate branch.

 EXCEPTION
     WHEN duplicate_object THEN 
-        -- Type already exists, check if it has the right values
-        RAISE NOTICE 'task_priority enum already exists, skipping creation';
+        -- Type exists: ensure labels present
+        BEGIN
+          ALTER TYPE task_priority ADD VALUE IF NOT EXISTS 'critical';
+          ALTER TYPE task_priority ADD VALUE IF NOT EXISTS 'high';
+          ALTER TYPE task_priority ADD VALUE IF NOT EXISTS 'medium';
+          ALTER TYPE task_priority ADD VALUE IF NOT EXISTS 'low';
+        EXCEPTION WHEN feature_not_supported THEN
+          RAISE NOTICE 'ALTER TYPE ADD VALUE not supported in this context; skipping label sync';
+        END;

88-90: Schema-qualify column existence check

Prevents false positives if multiple schemas exist.

-    IF EXISTS (SELECT 1 FROM information_schema.columns 
-               WHERE table_name = 'archon_tasks' AND column_name = 'priority') THEN
+    IF EXISTS (
+        SELECT 1 FROM information_schema.columns 
+        WHERE table_schema = 'public' AND table_name = 'archon_tasks' AND column_name = 'priority'
+    ) THEN

65-67: Consider concurrent index creation on large tables

CREATE INDEX can lock writes. If your migration runner allows out-of-transaction DDL, use CONCURRENTLY.

-- If your migration system supports it outside a transaction:
CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_archon_tasks_priority ON public.archon_tasks(priority);

Would you like me to generate a follow-up migration gated to only run when not in a transaction?

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 63d9a13 and d47c0e6.

📒 Files selected for processing (5)
  • archon-ui-main/src/features/projects/tasks/hooks/useTaskEditor.ts (2 hunks)
  • migration/add_priority_column_to_tasks.sql (1 hunks)
  • migration/complete_setup.sql (4 hunks)
  • python/src/server/api_routes/projects_api.py (5 hunks)
  • python/src/server/services/projects/task_service.py (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • python/src/server/api_routes/projects_api.py
  • migration/complete_setup.sql
  • archon-ui-main/src/features/projects/tasks/hooks/useTaskEditor.ts
🧰 Additional context used
📓 Path-based instructions (4)
python/src/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

python/src/**/*.py: Fail fast on service startup failures, missing configuration, database connection failures, authentication/authorization failures, critical dependencies unavailable, and invalid/corrupting data
Never accept or store corrupted data; on operation failure skip the item entirely rather than writing placeholders (e.g., zero embeddings)
For batch/background operations, continue processing but log each failure with details; track both successes and failures
Use specific exception types (avoid bare Exception), include context/IDs/URLs in messages, preserve full stack traces with logging (exc_info=True), and never return None/null to indicate failure—raise with details
Use database task status values directly: todo, doing, review, done
Target Python 3.12 style with 120-character line length; use Ruff for linting and Mypy for type checking

python/src/**/*.py: Fail fast on critical conditions: service startup failures, missing configuration/env vars, database connection/auth failures, critical dependencies unavailable
Never accept or store corrupted data (e.g., zero embeddings, null foreign keys, malformed JSON); skip failed items entirely and continue processing
For batch/background operations, continue processing but log detailed per-item failures; for external APIs use retries with exponential backoff and then fail clearly
Error messages must include context, use specific exception types, preserve full stack traces (logging with exc_info=True), include relevant IDs/URLs, and never return None to indicate failure—raise instead; for batch ops report success counts and detailed failures
Backend uses Python 3.12 with a 120-character line length
Avoid introducing WebSocket support in the backend; updates are handled via HTTP polling
Adhere to Ruff lint rules (e.g., no unused imports) and provide type hints to satisfy MyPy

Files:

  • python/src/server/services/projects/task_service.py
python/src/server/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Use FastAPI exception handlers to return rich error responses with appropriate HTTP status codes and typed error payloads

Use specific exception classes and FastAPI exception handlers to produce rich JSON error responses

Files:

  • python/src/server/services/projects/task_service.py
python/src/server/services/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Backend service method naming mirrors CRUD patterns: get/create/update/delete with clear resource scoping

Files:

  • python/src/server/services/projects/task_service.py
python/src/server/{api_routes,services}/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Follow Service Layer pattern: API routes delegate to services, which handle business logic and call the database layer

Files:

  • python/src/server/services/projects/task_service.py
🧠 Learnings (1)
📚 Learning: 2025-09-06T20:04:08.138Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T20:04:08.138Z
Learning: Applies to archon-ui-main/src/features/**/tasks/**/*.{ts,tsx} : Use task status values directly from the database: todo, doing, review, done (no UI mapping)

Applied to files:

  • migration/add_priority_column_to_tasks.sql
🧬 Code graph analysis (1)
python/src/server/services/projects/task_service.py (1)
python/src/server/api_routes/projects_api.py (1)
  • create_task (628-658)
🔇 Additional comments (5)
python/src/server/services/projects/task_service.py (5)

86-90: LGTM: priority validated before persistence

Validation path mirrors status/assignee checks and aligns with DB enum.


123-124: Persisting priority in task_data is correct

The field is included alongside task_order, matching the migration defaults.


148-149: Expose priority in response payload

Good addition; keeps API surface consistent with create/list/update.


186-187: Selecting priority in slim query is correct

Matches the new lightweight shape.


271-272: Safe defaulting on missing priority

Fallback to "medium" protects against partially migrated rows.

Comment thread migration/add_priority_column_to_tasks.sql
Comment thread migration/add_priority_column_to_tasks.sql
…lity)

Remove all task_order to priority mapping logic for true decoupling:

- All existing tasks get 'medium' priority (clean slate)
- No complex CASE logic or task_order relationships
- Users explicitly set priorities as needed after migration
- Truly independent priority and visual ordering systems
- Simpler, safer migration with no coupling logic

This approach prioritizes clean architecture over preserving
implied user intentions from the old coupled system.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
migration/add_priority_column_to_tasks.sql (1)

45-51: Don’t swallow NOT NULL/default enforcement errors — fail fast and pre-check NULLs

Catching WHEN OTHERS here hides real data problems and can leave the column nullable against app invariants. Let failures surface after an explicit NULL check.

Apply:

-            -- Ensure NOT NULL constraint (safe if already NOT NULL)
-            BEGIN
-                ALTER TABLE archon_tasks ALTER COLUMN priority SET NOT NULL;
-            EXCEPTION
-                WHEN OTHERS THEN
-                    RAISE NOTICE 'priority column already has NOT NULL constraint';
-            END;
-            
-            -- Ensure default value is set (safe if already set)
-            BEGIN
-                ALTER TABLE archon_tasks ALTER COLUMN priority SET DEFAULT 'medium';
-            EXCEPTION
-                WHEN OTHERS THEN
-                    RAISE NOTICE 'priority column already has default value';
-            END;
+            -- Enforce default and NOT NULL; fail fast if data invalid
+            ALTER TABLE archon_tasks ALTER COLUMN priority SET DEFAULT 'medium'::task_priority;
+            IF EXISTS (SELECT 1 FROM archon_tasks WHERE priority IS NULL) THEN
+                RAISE EXCEPTION 'Cannot SET NOT NULL: archon_tasks.priority still has NULLs';
+            END IF;
+            ALTER TABLE archon_tasks ALTER COLUMN priority SET NOT NULL;

Also applies to: 53-59

🧹 Nitpick comments (4)
migration/add_priority_column_to_tasks.sql (4)

17-24: Harden enum creation to handle legacy/missing labels

If environments previously used a different label (e.g., “urgent”) or are missing labels, normalize instead of only skipping on duplicate.

-DO $$ BEGIN
-    CREATE TYPE task_priority AS ENUM ('low', 'medium', 'high', 'critical');
-EXCEPTION
-    WHEN duplicate_object THEN 
-        -- Type already exists, check if it has the right values
-        RAISE NOTICE 'task_priority enum already exists, skipping creation';
-END $$;
+DO $$
+BEGIN
+  IF NOT EXISTS (SELECT 1 FROM pg_type WHERE typname = 'task_priority') THEN
+    CREATE TYPE task_priority AS ENUM ('low','medium','high','critical');
+  ELSE
+    -- Rename legacy value if present
+    IF EXISTS (
+      SELECT 1 FROM pg_enum e JOIN pg_type t ON t.oid = e.enumtypid
+      WHERE t.typname = 'task_priority' AND e.enumlabel = 'urgent'
+    ) THEN
+      ALTER TYPE task_priority RENAME VALUE 'urgent' TO 'critical';
+    END IF;
+    -- Ensure expected labels exist (best-effort for older PG)
+    BEGIN
+      ALTER TYPE task_priority ADD VALUE IF NOT EXISTS 'low';
+      ALTER TYPE task_priority ADD VALUE IF NOT EXISTS 'medium';
+      ALTER TYPE task_priority ADD VALUE IF NOT EXISTS 'high';
+      ALTER TYPE task_priority ADD VALUE IF NOT EXISTS 'critical';
+    EXCEPTION WHEN feature_not_supported THEN
+      -- No-op on older Postgres versions
+    END;
+  END IF;
+END $$;

83-85: Constrain information_schema lookup by schema to avoid false positives

Filter by table_schema (e.g., current_schema()) so similarly named tables in other schemas don’t bypass/trigger the block incorrectly.

-    IF EXISTS (SELECT 1 FROM information_schema.columns 
-               WHERE table_name = 'archon_tasks' AND column_name = 'priority') THEN
+    IF EXISTS (
+         SELECT 1
+         FROM information_schema.columns 
+         WHERE table_schema = current_schema()
+           AND table_name = 'archon_tasks'
+           AND column_name = 'priority'
+    ) THEN

76-98: Remove redundant “clean slate” block (NULL backfill already performed earlier)

You already backfill NULLs on Lines 31–35 and again in the duplicate_column path (Lines 43–44). This block will always update 0 rows and adds noise.

--- Set all existing tasks to default priority (clean slate approach)
--- This truly decouples priority from task_order - no relationship at all
-DO $$ 
-DECLARE 
-    updated_count INTEGER;
-BEGIN
-    -- Only proceed if priority column exists
-    IF EXISTS (SELECT 1 FROM information_schema.columns 
-               WHERE table_name = 'archon_tasks' AND column_name = 'priority') THEN
-        
-        -- Set all existing tasks to medium priority (clean slate)
-        -- Users can explicitly set priorities as needed after migration
-        UPDATE archon_tasks 
-        SET priority = 'medium'::task_priority
-        WHERE priority IS NULL;  -- Only update NULL values, preserve any existing priorities
-        
-        GET DIAGNOSTICS updated_count = ROW_COUNT;
-        RAISE NOTICE 'Set % existing tasks to medium priority (clean slate)', updated_count;
-    ELSE
-        RAISE NOTICE 'priority column does not exist, skipping initialization';
-    END IF;
-END $$;

29-29: Be explicit with enum casts for defaults

Use ::task_priority for clarity and to avoid relying on implicit casts.

-    ALTER TABLE archon_tasks ADD COLUMN priority task_priority DEFAULT 'medium';
+    ALTER TABLE archon_tasks ADD COLUMN priority task_priority DEFAULT 'medium'::task_priority;
@@
-                ALTER TABLE archon_tasks ALTER COLUMN priority SET DEFAULT 'medium';
+                ALTER TABLE archon_tasks ALTER COLUMN priority SET DEFAULT 'medium'::task_priority;

Also applies to: 55-55

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d47c0e6 and 4d0e732.

📒 Files selected for processing (1)
  • migration/add_priority_column_to_tasks.sql (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-06T20:04:08.138Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T20:04:08.138Z
Learning: Applies to archon-ui-main/src/features/**/tasks/**/*.{ts,tsx} : Use task status values directly from the database: todo, doing, review, done (no UI mapping)

Applied to files:

  • migration/add_priority_column_to_tasks.sql

Copy link
Copy Markdown
Collaborator

@Wirasm Wirasm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes looks great, will pull down and test before merging

…nsistency

- Renamed file to match the exported component name
- Updated import in index.ts barrel export
- Maintains consistency with other component naming patterns
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
archon-ui-main/src/features/projects/tasks/components/TaskPriorityComponent.tsx (1)

1-8: Header comment is outdated vs. PR scope (now server-backed).

Docstring still claims “frontend-only” and “until backend support is added.” Update to reflect the decoupled, persisted priority introduced in this PR.

 /**
  * TaskPriority Component
  *
- * Display-only priority selector for tasks.
- * NOTE: Priority is currently frontend-only and doesn't affect task ordering.
- * Task ordering is handled separately via drag-and-drop with task_order field.
- * This is purely for visual categorization until backend priority support is added.
+ * Server-backed priority selector for tasks.
+ * Priority is decoupled from drag-and-drop task_order.
+ * Levels: critical | high | medium | low.
  */
🧹 Nitpick comments (4)
archon-ui-main/src/features/projects/tasks/components/TaskPriorityComponent.tsx (4)

16-20: Prop name nit: consider isUpdatingPriority.

To match “is[Action]ing” guideline and improve clarity, rename isLoading → isUpdatingPriority (keep a temporary alias if needed).

-interface TaskPriorityProps {
-  priority?: TaskPriority;
-  onPriorityChange?: (priority: TaskPriority) => void;
-  isLoading?: boolean;
-}
+interface TaskPriorityProps {
+  priority?: TaskPriority;
+  onPriorityChange?: (priority: TaskPriority) => void;
+  isUpdatingPriority?: boolean;
+}

40-75: Make “low” explicit; default fallback to “medium” for future-proofing.

Avoid treating unknown future enum values as “low.”

-      default:
-        return {
-          background: glassmorphism.priority.low.background,
-          text: glassmorphism.priority.low.text,
-          hover: glassmorphism.priority.low.hover,
-          glow: glassmorphism.priority.low.glow,
-          iconColor: "text-gray-500",
-        };
+      case "low":
+        return {
+          background: glassmorphism.priority.low.background,
+          text: glassmorphism.priority.low.text,
+          hover: glassmorphism.priority.low.hover,
+          glow: glassmorphism.priority.low.glow,
+          iconColor: "text-gray-500",
+        };
+      default:
+        // Safe fallback if new enum values are introduced
+        return {
+          background: glassmorphism.priority.medium.background,
+          text: glassmorphism.priority.medium.text,
+          hover: glassmorphism.priority.medium.hover,
+          glow: glassmorphism.priority.medium.glow,
+          iconColor: "text-blue-500",
+        };

77-79: Avoid index-based fallback to “medium.”

Use a named default instead of PRIORITY_OPTIONS[2] which is brittle if order changes.

-const currentOption = PRIORITY_OPTIONS.find((opt) => opt.value === priority) || PRIORITY_OPTIONS[2]; // Default to medium
+const DEFAULT_PRIORITY: TaskPriority = "medium";
+const currentOption =
+  PRIORITY_OPTIONS.find((opt) => opt.value === priority) ??
+  PRIORITY_OPTIONS.find((opt) => opt.value === DEFAULT_PRIORITY)!;

102-104: Avoid type assertion on onValueChange; add a type guard.

Stay within TS strict without casts.

-<Select value={priority} onValueChange={(value) => onPriorityChange(value as TaskPriority)}>
+<Select
+  value={priority}
+  onValueChange={(value) => {
+    if (isTaskPriority(value)) onPriorityChange(value);
+  }}
+>

Add this helper (top-level in the module):

const isTaskPriority = (v: string): v is TaskPriority =>
  (["critical", "high", "medium", "low"] as const).includes(v as TaskPriority);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4d0e732 and 68839c1.

📒 Files selected for processing (2)
  • archon-ui-main/src/features/projects/tasks/components/TaskPriorityComponent.tsx (3 hunks)
  • archon-ui-main/src/features/projects/tasks/components/index.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • archon-ui-main/src/features/projects/tasks/components/index.ts
🧰 Additional context used
📓 Path-based instructions (3)
archon-ui-main/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

archon-ui-main/src/**/*.{ts,tsx}: Use TanStack Query for all data fetching; avoid prop drilling
TypeScript: strict mode with no implicit any in frontend code
State naming: is[Action]ing for loading flags, [resource]Error for errors, selected[Resource] for current selection
Use HTTP polling with ETag caching; do not introduce WebSocket-based updates in the frontend

archon-ui-main/src/**/*.{ts,tsx}: WebSocket event failures (if any) should be logged and not crash the client; continue serving others
Frontend data fetching must use TanStack Query (no prop drilling) with query key factories, smart polling, and optimistic updates with rollback
Use vertical slice architecture: place UI under src/features/[feature]/(components|hooks|services|types)
State naming: use is[Action]ing for loading, [resource]Error for errors, selected[Resource] for selections
Service method names: get[Resource]sByProject(projectId), getResource, create/update/delete patterns
Frontend TypeScript should be strict (no implicit any)

Files:

  • archon-ui-main/src/features/projects/tasks/components/TaskPriorityComponent.tsx
archon-ui-main/src/features/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

archon-ui-main/src/features/**/*.{ts,tsx}: Follow TanStack Query patterns: query-keys factory, smart polling via useSmartPolling, optimistic updates with rollback
Biome formatting in features: 120-character lines, double quotes, trailing commas

archon-ui-main/src/features/**/*.{ts,tsx}: Use Biome formatting/conventions in /src/features: 120-char lines, double quotes, trailing commas
Use useSmartPolling and polling intervals (1–2s active, 5–10s background) with smart pausing on tab inactivity
Expose progress via dedicated hooks (e.g., useCrawlProgressPolling, useProjectTasks) instead of ad-hoc timers
Do not use prop drilling for data fetching/state; rely on TanStack Query caches/selectors

Files:

  • archon-ui-main/src/features/projects/tasks/components/TaskPriorityComponent.tsx
**/*.{py,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Never return None/null to indicate failure; raise an exception with details instead

Files:

  • archon-ui-main/src/features/projects/tasks/components/TaskPriorityComponent.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-13T15:53:40.741Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T15:53:40.741Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Expose progress via dedicated hooks (e.g., useCrawlProgressPolling, useProjectTasks) instead of ad-hoc timers

Applied to files:

  • archon-ui-main/src/features/projects/tasks/components/TaskPriorityComponent.tsx
🧬 Code graph analysis (1)
archon-ui-main/src/features/projects/tasks/components/TaskPriorityComponent.tsx (2)
archon-ui-main/src/features/projects/tasks/components/index.ts (1)
  • TaskPriorityComponent (16-16)
archon-ui-main/src/features/ui/primitives/select.tsx (1)
  • Select (7-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Backend Tests (Python + pytest)
🔇 Additional comments (4)
archon-ui-main/src/features/projects/tasks/components/TaskPriorityComponent.tsx (4)

14-14: Type-only import: LGTM.


24-24: Typed option values: LGTM.


116-116: Resolved — SelectTrigger consumes showChevron. It's declared and destructured (not forwarded via ...props) and used internally to render the chevron, so no React unknown-prop warnings.


34-38: Component rename verified — no stale usages or imports remain.

index.ts exports TaskPriorityComponent and usages are only (e.g., TaskCard.tsx); no JSX or imports referencing a TaskPriority filename were found.

@Wirasm Wirasm merged commit c45842f into main Sep 17, 2025
8 checks passed
leonj1 pushed a commit to leonj1/Archon that referenced this pull request Oct 13, 2025
* feat: decouple task priority from task order

This implements a dedicated priority system that operates independently
from the existing task_order system, allowing users to set task priority
without affecting visual drag-and-drop positioning.

## Changes Made

### Database
- Add priority column to archon_tasks table with enum type (critical, high, medium, low)
- Create database migration with safe enum handling and data backfill
- Add proper indexing for performance

### Backend
- Update UpdateTaskRequest to include priority field
- Add priority validation in TaskService with enum checking
- Include priority field in task list responses and ETag generation
- Fix cache invalidation for priority updates

### Frontend
- Update TaskPriority type from "urgent" to "critical" for consistency
- Add changePriority method to useTaskActions hook
- Update TaskCard to use direct priority field instead of task_order conversion
- Update TaskEditModal priority form to use direct priority values
- Fix TaskPriorityComponent to use correct priority enum values
- Update buildTaskUpdates to include priority field changes
- Add priority field to Task interface as required field
- Update test fixtures to include priority field

## Key Features
- ✅ Users can change task priority without affecting drag-and-drop order
- ✅ Users can drag tasks to reorder without changing priority level
- ✅ Priority persists correctly in database with dedicated column
- ✅ All existing priority functionality continues working identically
- ✅ Cache invalidation works properly for priority changes
- ✅ Both TaskCard priority button and TaskEditModal priority work

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* feat: add priority column to complete_setup.sql for fresh installations

- Add task_priority enum type (low, medium, high, critical)
- Add priority column to archon_tasks table with default 'medium'
- Add index for priority column performance
- Add documentation comment for priority field

This ensures fresh installations include the priority system without
needing to run the separate migration.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: include priority field in task creation payload

When creating new tasks via TaskEditModal, the buildCreateRequest function
was not including the priority field, causing new tasks to fall back to
the database default ('medium') instead of respecting the user's selected
priority in the modal.

Added priority: localTask.priority || 'medium' to ensure the user's
chosen priority is sent to the API during task creation.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* feat: make priority migration safe and idempotent

Replaced destructive DROP TYPE CASCADE with safe migration patterns:

- Use DO blocks with EXCEPTION handling for enum and column creation
- Prevent conflicts with complete_setup.sql for fresh installations
- Enhanced backfill logic to preserve user-modified priorities
- Only update tasks that haven't been modified (updated_at = created_at)
- Add comprehensive error handling with informative notices
- Migration can now be run multiple times safely

This ensures the migration works for both existing installations
(incremental migration) and fresh installations (complete_setup.sql)
without data loss or conflicts.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* feat: enforce NOT NULL constraint on priority column

Data integrity improvements:

Migration (add_priority_column_to_tasks.sql):
- Add column as nullable first with DEFAULT 'medium'
- Update any NULL values to 'medium'
- Set NOT NULL constraint to enforce application invariants
- Safe handling for existing columns with proper constraint checking

Complete Setup (complete_setup.sql):
- Priority column now DEFAULT 'medium' NOT NULL for fresh installations
- Ensures consistency between migration and fresh install paths

Both paths now enforce priority field as required, matching the
frontend Task interface where priority is a required field.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* feat: add priority support to task creation API

Complete priority support for task creation:

API Routes (projects_api.py):
- Add priority field to CreateTaskRequest Pydantic model
- Pass request.priority to TaskService.create_task call

Task Service (task_service.py):
- Add priority parameter to create_task method signature
- Add priority validation using existing validate_priority method
- Include priority field in database INSERT task_data
- Include priority field in API response task object

This ensures that new tasks created via TaskEditModal respect the
user's selected priority instead of falling back to database default.

Validation ensures only valid priority values (low, medium, high, critical)
are accepted and stored in the database.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* feat: implement clean slate priority migration (no backward compatibility)

Remove all task_order to priority mapping logic for true decoupling:

- All existing tasks get 'medium' priority (clean slate)
- No complex CASE logic or task_order relationships
- Users explicitly set priorities as needed after migration
- Truly independent priority and visual ordering systems
- Simpler, safer migration with no coupling logic

This approach prioritizes clean architecture over preserving
implied user intentions from the old coupled system.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* refactor: rename TaskPriority.tsx to TaskPriorityComponent.tsx for consistency

- Renamed file to match the exported component name
- Updated import in index.ts barrel export
- Maintains consistency with other component naming patterns

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Rasmus Widing <rasmus.widing@gmail.com>
@Wirasm Wirasm deleted the feature/decouple-task-priority-from-order branch April 6, 2026 07:37
coleam00 added a commit that referenced this pull request Apr 7, 2026
Move tool call persistence from WebAdapter's MessagePersistence to the
workflow executor via workflow_events table. Tool calls are now persisted
as 'tool_called' events in all three execution paths (sequential, loop,
DAG) using the existing fire-and-forget pattern, making them visible in
the web UI regardless of which adapter initiated the workflow.

- Add 'tool_called' to WorkflowEventType union
- Persist tool events in executor (sequential + loop) and dag-executor
- Update WorkflowExecution to extract tool events from workflow_events
- Update WorkflowLogs to render tool calls from events instead of
  message metadata
- Simplify WebAdapter.sendStructuredEvent (remove persistence, keep SSE)
- Remove appendToolCall/finalizeRunningTools from MessagePersistence
- Add tests for tool_called event persistence across all execution modes
Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026
…) (coleam00#652)

Move tool call persistence from WebAdapter's MessagePersistence to the
workflow executor via workflow_events table. Tool calls are now persisted
as 'tool_called' events in all three execution paths (sequential, loop,
DAG) using the existing fire-and-forget pattern, making them visible in
the web UI regardless of which adapter initiated the workflow.

- Add 'tool_called' to WorkflowEventType union
- Persist tool events in executor (sequential + loop) and dag-executor
- Update WorkflowExecution to extract tool events from workflow_events
- Update WorkflowLogs to render tool calls from events instead of
  message metadata
- Simplify WebAdapter.sendStructuredEvent (remove persistence, keep SSE)
- Remove appendToolCall/finalizeRunningTools from MessagePersistence
- Add tests for tool_called event persistence across all execution modes
joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026
…) (coleam00#652)

Move tool call persistence from WebAdapter's MessagePersistence to the
workflow executor via workflow_events table. Tool calls are now persisted
as 'tool_called' events in all three execution paths (sequential, loop,
DAG) using the existing fire-and-forget pattern, making them visible in
the web UI regardless of which adapter initiated the workflow.

- Add 'tool_called' to WorkflowEventType union
- Persist tool events in executor (sequential + loop) and dag-executor
- Update WorkflowExecution to extract tool events from workflow_events
- Update WorkflowLogs to render tool calls from events instead of
  message metadata
- Simplify WebAdapter.sendStructuredEvent (remove persistence, keep SSE)
- Remove appendToolCall/finalizeRunningTools from MessagePersistence
- Add tests for tool_called event persistence across all execution modes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants