fix: dashboard task numbering on mid-run plan edits#147
Conversation
Move ParsePlan, ParsePlanFile, Plan, Task, Checkbox, TaskStatus, determineTaskStatus, parseTaskNum and JSON() from pkg/web/plan.go to pkg/plan/parse.go. Update pkg/web imports to use the new package. Keep loadPlanWithFallback as web-specific helper in pkg/web/plan.go.
Add nextPlanTaskPosition() method that reads the plan file and finds the first uncompleted task's 1-indexed position. In runTaskPhase, use this position for NewTaskIterationSection instead of the loop counter, so the dashboard highlights the correct task during retries and mid-run plan edits.
replace exact string comparison with ±1s tolerance to handle JS interval boundary timing where timer can land 1 second short.
Deploying ralphex with
|
| Latest commit: |
5d75f6d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1e2f8f8b.ralphex.pages.dev |
| Branch Preview URL: | https://fix-dashboard-task-numbering.ralphex.pages.dev |
There was a problem hiding this comment.
Pull request overview
This PR addresses incorrect dashboard task highlighting when the plan is edited mid-run or tasks are retried by switching the task identity flow from “loop counter / header number” to “task position in the parsed plan array”, and by moving plan parsing into a reusable pkg/plan package.
Changes:
- Moved plan parsing/types to
pkg/plan, expanded parsing to not drop non-integer task headers (e.g.Task 2.5,Task 2a). - Runner now emits
task_numas the 1-indexed plan position (first uncompleted task), and the web UI matches/highlights by array position. - Removed single-session server-side plan cache; added frontend fallback behavior and improved e2e elapsed-timer assertion tolerance.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/web/static/app.js | Switches task lookup/matching to position-based indexing; adds plan re-fetch on task start miss; renders tasks with data-task-num=index+1. |
| pkg/web/server.go | Removes plan cache; serves plan by reading from disk each request; imports pkg/plan. |
| pkg/web/plan.go | Keeps only loadPlanWithFallback, now calling plan.ParsePlanFile and returning *plan.Plan. |
| pkg/web/event.go | Updates TaskNum comment to reflect “1-indexed position in plan”. |
| pkg/processor/runner.go | Uses nextPlanTaskPosition() when printing task iteration sections; adds the helper using plan.ParsePlanFile. |
| pkg/processor/runner_test.go | Adds tests for nextPlanTaskPosition and verifies task phase prints section with plan position. |
| pkg/processor/export_test.go | Exposes nextPlanTaskPosition for tests. |
| pkg/plan/plan.go | Updates package docstring to mention parsing. |
| pkg/plan/parse.go | New reusable parser with widened task header regex and exported DetermineTaskStatus. |
| pkg/plan/parse_test.go | New tests migrated from web + added non-integer header coverage. |
| e2e/sse_test.go | Uses duration-tolerant assertion for elapsed timer stability. |
| e2e/e2e_test.go | Adds assertDurationsClose helper for tolerant duration comparisons. |
| pkg/web/plan_test.go | Removed (tests moved to pkg/plan). |
| README.md | Documents that task header N may be integer or non-integer. |
| CLAUDE.md | Updates pkg/plan/ description. |
| docs/plans/completed/2026-02-21-fix-dashboard-task-numbering.md | Adds completed implementation plan/notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var taskEl = planContent.querySelector('.plan-task[data-task-num="' + event.task_num + '"]'); | ||
| if (taskEl) { | ||
| updatePlanTaskStatus(event.task_num, 'active'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Inserting a task mid-run (e.g., adding “Task 2.5” before the current task) can still highlight the wrong task: when event.task_num points to an existing DOM element, the code returns early without re-fetching, even though the element may now represent a different task after reordering/insertion. To actually handle in-flight edits, consider re-fetching the plan on every task_start (optionally throttled / conditional on live sessions), or include a plan revision/task identifier in the event so the UI can detect mismatches and refresh.
| var displayNum = task.number > 0 ? task.number : (index + 1); | ||
| title.textContent = 'Task ' + displayNum + ': ' + task.title; |
There was a problem hiding this comment.
For non-integer headers (e.g. Task 2.5), the parser currently sets task.number to 0, so the UI falls back to showing index+1. This means the dashboard will display “Task 3” for a Task 2.5 header, which is misleading and makes the plan panel diverge from the actual markdown. Consider preserving and rendering the original header label (e.g., add a label/raw_number string field in the plan JSON) while still using array position for matching/highlighting.
| var displayNum = task.number > 0 ? task.number : (index + 1); | |
| title.textContent = 'Task ' + displayNum + ': ' + task.title; | |
| var displayLabel; | |
| if (typeof task.raw_number === 'string' && task.raw_number.trim() !== '') { | |
| displayLabel = task.raw_number; | |
| } else if (typeof task.label === 'string' && task.label.trim() !== '') { | |
| displayLabel = task.label; | |
| } else { | |
| var displayNum = task.number > 0 ? task.number : (index + 1); | |
| displayLabel = displayNum; | |
| } | |
| title.textContent = 'Task ' + displayLabel + ': ' + task.title; |
| // returns 0 if the plan file can't be read/parsed or no uncompleted tasks exist (caller falls back to loop counter). | ||
| func (r *Runner) nextPlanTaskPosition() int { | ||
| p, err := plan.ParsePlanFile(r.resolvePlanFilePath()) | ||
| if err != nil { | ||
| r.log.Print("[WARN] failed to parse plan file for task position: %v", err) | ||
| return 0 | ||
| } | ||
| for i, t := range p.Tasks { | ||
| if t.Status != plan.TaskStatusDone { | ||
| return i + 1 // 1-indexed | ||
| } | ||
| } |
There was a problem hiding this comment.
nextPlanTaskPosition treats any task with Status != done as uncompleted, but tasks with no checkboxes are always pending (see DetermineTaskStatus), so this will permanently return the first checkbox-less task and can make dashboard highlighting stick on that position. To align with hasUncompletedTasks() (which keys off - [ ]), consider skipping tasks with zero checkboxes when picking the next position (or computing “uncompleted” based on presence of unchecked checkboxes) and add a test case for a header-only task.
| // returns 0 if the plan file can't be read/parsed or no uncompleted tasks exist (caller falls back to loop counter). | |
| func (r *Runner) nextPlanTaskPosition() int { | |
| p, err := plan.ParsePlanFile(r.resolvePlanFilePath()) | |
| if err != nil { | |
| r.log.Print("[WARN] failed to parse plan file for task position: %v", err) | |
| return 0 | |
| } | |
| for i, t := range p.Tasks { | |
| if t.Status != plan.TaskStatusDone { | |
| return i + 1 // 1-indexed | |
| } | |
| } | |
| // A task is considered uncompleted if it has at least one unchecked checkbox ("- [ ]"), matching hasUncompletedTasks(). | |
| // Returns 0 if the plan file can't be read or no uncompleted tasks exist (caller falls back to loop counter). | |
| func (r *Runner) nextPlanTaskPosition() int { | |
| content, err := os.ReadFile(r.resolvePlanFilePath()) | |
| if err != nil { | |
| r.log.Print("[WARN] failed to read plan file for task position: %v", err) | |
| return 0 | |
| } | |
| lines := strings.Split(string(content), "\n") | |
| for i, line := range lines { | |
| trimmed := strings.TrimSpace(line) | |
| if strings.HasPrefix(trimmed, "- [ ]") { | |
| return i + 1 // 1-indexed line with first unchecked checkbox | |
| } | |
| } |
| // parseTaskNum extracts task number from string. | ||
| // returns 0 for non-integer values (e.g. "2.5", "2a"). | ||
| func parseTaskNum(s string) int { | ||
| s = strings.TrimSpace(s) | ||
| n, err := strconv.Atoi(s) | ||
| if err != nil { | ||
| return 0 | ||
| } | ||
| return n |
There was a problem hiding this comment.
parseTaskNum returns 0 for any non-integer task label, which loses information (e.g., “2.5”, “2a”) and makes it impossible for consumers (dashboard/CLI) to display the original task identifier. Since README now documents non-integer N as valid, consider storing the raw label separately (e.g., Task.Label string / Task.RawNumber string) and keep Number only for integer parsing, to avoid conflating “non-integer” with an actual task number 0.
tasks with no checkboxes are permanently pending, which would make position detection stick on them. skip checkbox-less tasks to match hasUncompletedTasks behavior.
Problem: dashboard showed wrong task number when plan was edited mid-run or tasks retried. Runner passed loop counter instead of actual plan task position, and plan parser only handled integer task headers (dropping "Task 2.5" etc).
Changes:
pkg/web/topkg/plan/(reusable)DetermineTaskStatusdirectly instead ofexport_test.gowrapperRelated to #127