fleet: taskSorting + TaskReviewTab 8 → 0 — and Board was faking a column id to force done-sorting

[gsxdsm]

Two app-side clusters, 8 → 0, plus a caller-side hack retired.

What was broken

TaskReviewTab.tsx — three of its four questions were task.column === "in-review", driving the Create-PR button, the "frozen on entry to review" auto-merge hint, and PR-feedback addressing. On a renamed review lane all three took their non-review branch: the button was absent, and the hint claimed the effective auto-merge value was not frozen when it was.

taskSorting.ts — isReviewColumn decides whether merging cards float to the top of a lane. Keyed on the id it silently stopped doing that on any renamed review lane, so the operator loses the "what is merging right now" ordering with nothing failing.

Both follow the shape this code already established: caller supplies the trait, default to the legacy id. columnFlags on the review tab is optional and wired from TaskDetailModal, which already resolved it for canEdit and the actions menu.

A synthetic column id, retired

Board.tsx forced done-sorting by passing the literal "done" as the column argument for any complete-flagged lane:

grouped[column.id] = isWorkflowDoneLikeColumn
  ? sortTasksForDisplayColumn(grouped[column.id] ?? [], "done", doneSortMode)
  : sortTasksForDisplayColumn(grouped[column.id] ?? [], column.id as ColumnType, ...);

A synthetic id standing in for a trait — so a custom complete lane sorted correctly only because its caller lied about its name. Both call sites now pass the real column id and state the trait. (Board's own census count stays at 2: those two literals were the synthetic ids and are gone; the 2 remaining are different sites.)

Revert proof

reverted	failure
task.column === "in-review" on the Create-PR guard	Unable to find an element by: [data-testid="task-review-create-pr"]
same, on the auto-merge hint	expected 'Effective: Auto-merge off' to contain 'frozen on entry to review'

A third case pins that the widened test does not treat every column as review.

None of the 45 existing TaskReviewTab cases could have caught this — columnFlags is optional and they all omit it, so they assert the legacy fallback. That is the same blind spot as the reconciler's 33 in #2737, and it keeps recurring: an optional-flags seam means the existing suite stays green through the conversion and through a broken one.

A process failure worth recording

I lost this conversion once and had to redo it. I overwrote four files with their origin/main versions to check whether a failing test was pre-existing, then "restored" with git checkout HEAD -- <dir>. HEAD was still origin/main because I had not committed, so that discarded the work.

Same class as the shared-stash incident two PRs back: an implicit or positional restore reference. The fix is ordering, not care — commit before any baseline comparison, so git checkout HEAD -- <file> restores my work rather than main's. This PR's commit was created before the comparison for exactly that reason, and the note is in the commit message so the next person hits it there too.

Verification

pnpm test:gate GREEN (158 + 10 + 487 + 71) · 232 passed across TaskReviewTab / taskSorting / Board suites · dashboard tsc -p tsconfig.app.json clean · pnpm lint clean · census --strict exits 0.

The 1 board-mobile failure is pre-existing — verified by swapping in clean origin/main copies of all four files and reproducing it.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

@gsxdsm, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 23 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 18416672-9a5a-4f1c-a462-b7a21bea3276

📥 Commits

Reviewing files that changed from the base of the PR and between ceca08b and 781385f.

📒 Files selected for processing (8)

• packages/dashboard/app/components/Board.tsx
• packages/dashboard/app/components/TaskCard.tsx
• packages/dashboard/app/components/TaskDetailModal.tsx
• packages/dashboard/app/components/TaskReviewTab.tsx
• packages/dashboard/app/components/__tests__/TaskReviewTab.test.tsx
• packages/dashboard/app/components/taskSorting.ts
• packages/dashboard/app/utils/prFeedback.ts
• scripts/lib/lifecycle-column-census-baseline.json

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR consistently replaces legacy workflow-column ID checks with resolved role flags.

• Passes task-matched column flags into the review tab and PR-feedback eligibility helper.
• Uses explicit complete and review traits when sorting board columns while preserving legacy-ID defaults for unresolved callers.
• Adds coverage for renamed review lanes and updates the lifecycle-column census baseline.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the current identity guard prevents cross-task flags from reaching the review tab, and PR-feedback eligibility now uses the same resolved role flags as its callers.

Important Files Changed

Filename	Overview
packages/dashboard/app/components/TaskDetailModal.tsx	Passes identity-guarded column flags to TaskReviewTab, resolving the previously reported cross-task stale-flags issue.
packages/dashboard/app/components/TaskReviewTab.tsx	Replaces legacy review and WIP column-ID predicates with role-based checks for review actions and auto-merge messaging.
packages/dashboard/app/utils/prFeedback.ts	Extends PR-feedback eligibility to use resolved review and WIP traits while retaining legacy-ID fallback behavior.
packages/dashboard/app/components/TaskCard.tsx	Supplies resolved task-column flags when evaluating the PR-feedback action.
packages/dashboard/app/components/taskSorting.ts	Makes complete and review sorting explicit traits rather than inferring them solely from built-in column IDs.
packages/dashboard/app/components/Board.tsx	Passes real column IDs and resolved sorting traits instead of synthesizing the done column ID.
packages/dashboard/app/components/tests/TaskReviewTab.test.tsx	Adds regression coverage for renamed review lanes and confirms non-review lanes remain excluded.
scripts/lib/lifecycle-column-census-baseline.json	Removes converted legacy-column references from the census baseline.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    Workflow[Resolved workflow column] --> Flags[Column role flags]
    Flags --> Modal[TaskDetailModal identity guard]
    Modal --> ReviewTab[TaskReviewTab]
    Flags --> Card[TaskCard]
    ReviewTab --> Feedback[PR-feedback eligibility]
    Card --> Feedback
    Flags --> Board[Board sorting]
    Board --> Sorter[Task display sorting]

Loading

_{Reviews (6): Last reviewed commit: "chore(census): re-record baseline after ..." | Re-trigger Greptile}

[gsxdsm]

Both findings are real. I had fixes for both and the branch was taken by another worktree mid-change, so I am handing over the analysis rather than fighting for the ref. Everything below is verified against the current file contents, including one trap that cost me two failing tests.

Finding 1 — stale flags cross tasks (TaskDetailModal.tsx:5474)

Real, and the fix already exists in the file. Line 961 computes the guarded value:

const detailFlagsAreForThisTask = workflowMoveMetadata?.taskId === task.id;
const detailColumnFlags = detailFlagsAreForThisTask ? workflowMoveMetadata?.currentColumnFlags : undefined;

The review-tab prop reads around it, passing workflowMoveMetadata?.currentColumnFlags raw. Since that metadata outlives a task switch, switching tasks with the review tab open feeds the previous task's flags for a render — and this tab gates Create PR, Address PR Feedback and the frozen auto-merge hint on them. The one-word fix is columnFlags={detailColumnFlags}.

Five more unguarded reads of the same value exist, which the finding does not name: lines 1822 (canEdit), 2142 and 2280 (requiresExecutionModeReplan), 3151 (isIntakeColumn), 3777. Each needs judging separately — the two in save handlers may be fine since they run against the current task — but they are the enumeration list, and #2696 is currently fixing a sixth instance in this same file (handleDelete's dep array). That is six findings across one file for one root cause; it deserves one pass rather than six rounds.

Finding 2 — canStartPrFeedbackAddressing (TaskReviewTab.tsx:214)

Real. The caller checks resolved roles and then ANDs with a predicate requiring the literal pair:

&& (isReviewColumn || isWipColumn)
&& (canStartPrFeedbackAddressing(task) || displayItems.length > 0);

Why it hides: the || displayItems.length > 0 means the action still appears once review items load, so the disagreement looks like a loading delay. The gap only shows for a card with actionable feedback and nothing loaded yet — exactly when an operator wants the button.

Fix shape that matches the pattern already settled elsewhere (#2736's classifier, #2728): an optional resolved boolean, defaulting to the legacy pair so TaskCard.tsx:2002 — the other caller — is unchanged.

export function canStartPrFeedbackAddressing(
  task: Pick<Task, "column" | "prInfo" | "prInfos">,
  isReviewOrWipLane?: boolean,
): boolean {
  return (isReviewOrWipLane ?? (task.column === "in-review" || task.column === "in-progress"))
    && hasActionablePrFeedback(task);
}

…called as canStartPrFeedbackAddressing(task, isReviewColumn || isWipColumn).

The trap, worth having before you write the test. This predicate has no test file, and the caller's 48 tests pass with the resolved answer ignored — so a new test is required, and its fixture is easy to get wrong. hasActionablePrFeedback reads:

(prInfo.commentCount ?? 0) > 0 || prInfo.lastReviewDecision === "CHANGES_REQUESTED"

I built a fixture with reviewCommentCount / addressedReviewCommentCount — plausible names, wrong ones — and got two failures that looked like the fix was broken when the fixture was. Use commentCount or lastReviewDecision.

My work is stashed locally and not pushed; nothing of mine is on the branch.

[gsxdsm]

Both P1s were real and are fixed in 0e4f2e0-onward. Neither was a false positive.

1. Stale column flags across tasks — correct, and embarrassing. I passed workflowMoveMetadata?.currentColumnFlags raw. This file already has detailColumnFlags, which applies detailFlagsAreForThisTask (workflowMoveMetadata?.taskId === task.id) precisely because on the render where the modal switches tasks the state still holds the previous card's payload. Passing the raw value resolves the review tab's roles from another task's workflow — confidently wrong rather than merely stale. That reasoning was already written six lines above my own change and I reviewed past it. Taking the suggested columnFlags={detailColumnFlags} verbatim.

2. Legacy predicate blocks feedback addressing — correct, and it was a half-conversion I shipped. canStartPrFeedbackAddressing in utils/prFeedback.ts carries the same lane pair the tab asks about, and I converted only the caller. On a renamed review or WIP lane, a task with actionable PR feedback but no loaded display items kept the action hidden: the caller's role check passed and the helper returned false. It now takes optional flags on the same seam, with both callers (TaskReviewTab, TaskCard) supplying what they already hold.

That is the third instance of "both halves or neither" in this program — the archived gate (#2724), the tracking pair (#2715), and now a lane pair split between a component and its util. Converting the visible half first is exactly what makes the remaining half invisible.

Revert proof

Restoring the id pair in canStartPrFeedbackAddressing fails the new case. It deliberately supplies empty review items — with items present the displayItems.length > 0 arm masks the helper and the bug cannot be observed at all.

Two fixture details my first attempt got wrong, now recorded in the test so the next reader does not repeat them: the field actually read is lastReviewDecision (not reviewDecision), and isPrMode requires review source pull-request.

Verification

Gate GREEN (158 + 10 + 487 + 71) · 463 passed across review-tab / card / sorting / columnRoles · census --update-baseline reported exactly "TIGHTENED 1 entry" (prFeedback 2 → 0) · dashboard tsc and pnpm lint clean.

The 2 TaskCard failures are the known pre-existing CSS-var geometry assertions, verified against clean origin/main copies.

[gsxdsm]

Rebased onto current main; MERGEABLE. Both findings are addressed on the branch and I verified them rather than trusting the commit messages.

Finding 1 — the review tab now receives detailColumnFlags, the task-identity-guarded value the file already computed, instead of the raw workflowMoveMetadata.

Finding 2 — canStartPrFeedbackAddressing takes resolved flags. Worth noting the owner's shape is better than the one I proposed in my earlier comment here: I suggested an optional boolean (matching #2736's classifier), but taking ColumnRoleFlags and calling the role helpers inside keeps the role logic in one place rather than making every caller pre-compute the same disjunction. I withdraw the boolean suggestion.

Verified on the rebased tree: dashboard tsc clean (tsconfig.app.json), gate green (487 + 158 + 10 + 71), census --strict exit 0. Force-push checked with --not HEAD: 4 remote commits, 4 reproduced.

One note for whoever picks up the follow-up I flagged earlier — the five remaining unguarded reads of workflowMoveMetadata?.currentColumnFlags (lines 1822, 2142, 2280, 3151, 3777) are still there, and #2696 is fixing a sixth instance in the same file. That is six findings for one root cause; a single pass over the file would retire the class.