fix(dashboard): the high fan-out blocker warning never rendered on a renamed board - #2991
Merged
gsxdsm merged 1 commit intoJul 31, 2026
Conversation
…renamed board Second surface of the fan-out defect fixed in the parent commit; this one is the executor status bar, and the answer was already in scope. `ExecutorStatusBar` computes its own fan-out map to find the blocker holding up the most work. It passed only `staleHighFanoutAgeThresholdMs`, so core fell back to `holdColumn: "todo"` — a lane a renamed board does not have. `overlapBlockedTodoCount` stayed at zero, the high-fan-out threshold (>= 5) never tripped, and the warning simply never appeared while cards sat blocked behind one card. The flags were in scope the whole time: this component already receives `columnFlagsByTaskId` and hands it to `useExecutorStats`. It just never reached the fan-out. ONE TYPE FIX RIDES ALONG, and it is not cosmetic. `ExecutorColumnFlags` was a Pick that dropped `humanReview`, while its supplier in App.tsx builds the map from `workflow.columns.find(...).flags` — the whole object — so the trait is present at RUNTIME and only the type discarded it. Harmless while these flags answered complete/archived/wip questions; not harmless for the review role, because `isReviewColumnRole` reads `mergeBlocker || humanReview`, so a lane hosting a human review WITHOUT blocking merges would have classified as not-review. That is a wrong answer rather than a degradation — the "supplying the wrong flags" shape — so the Pick now declares what the data already has. MEASURED - removing the two classifiers: the renamed case fails, control and negative both pass - ExecutorStatusBar suite + both new fan-out suites: 76 tests green - all five gates green; lint and tsc clean The negative case is the one worth keeping: cards already in the renamed WIP lane must not count as held, or the fix trades a silent zero for a wrong number, which invites no scrutiny at all. STILL NOT DONE: TaskDetailModal. It holds `detailColumnFlags` for the OPEN task only, and the fan-out needs a role answer for every DEPENDENT, so unlike this surface the data is genuinely absent rather than merely unthreaded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
This was referenced Jul 31, 2026
gsxdsm
added a commit
that referenced
this pull request
Jul 31, 2026
#2993) > Against #2990's branch. This is me folding #2989 into yours rather than running two PRs over the same seam — see [my comment there](#2990 (comment)). ## The Board half of #2990 is inert `useBlockerFanout` memoizes on: ```ts [tasks, options.staleHighFanoutAgeThresholdMs] ``` `columnFlagsByTaskId` isn't in the list. `Board` builds that index from `boardWorkflows`, which is **null until an async fetch resolves** — so the first computation runs against an empty map and takes the documented legacy fallback. When the index populates, neither dependency has changed, the memo never recomputes, and the pre-load answer survives for the life of the mount. The `flagsByTaskId.size > 0` guard is right. It only ever ran against the empty map. Threaded end to end, correctly typed, and never arriving — the first failure shape from the learnings doc, in the one place lint can't see it: **this repo has no `react-hooks/exhaustive-deps` rule**, and a disable directive for it fails CI, so the dep list is maintained by hand. ## Measured | state | result | |---|---| | this branch before the commit | `expected +0 to be 2` | | after | 2 passed | | `useBlockerFanout` + `ExecutorStatusBar` + new Board suite | **87 tests green** | Census and FNXC gates green; lint and `tsc` clean. ## The test is the point It drives the **real `Board`** with a mocked `Column` that captures the map it was handed. The existing suites call `computeBlockerFanoutMap` directly and pass a populated map from the first call — so they exercise the pure function and never the memo, which is exactly where the value has to survive an async arrival. That's why a correct implementation and green tests coexisted with a board that read legacy lanes. It also asserts the tree actually rendered before trusting the captured map: a Board that throws mid-render yields `undefined` rather than a wrong number, and I'd rather that fail loudly than read as a passing zero. ## What I'm dropping #2989 and its stacked #2991 are superseded by yours — I'll close #2989 once you've taken what you want. Yours is broader (`reviewColumns`, the `mergeOrchestration` arm, scheduler parity, the census baseline), so this carries over only the two things it didn't have: the memo dep and the producer-level test. Also withdrawn: I claimed on #2991 that the `ExecutorColumnFlags` `Pick` dropping `humanReview` produced a wrong answer. It doesn't — the sole supplier (`App.tsx:553`) passes the whole `column.flags` object, so the trait is there at runtime. Latent type-safety at most; your type is fine as-is. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect
ExecutorStatusBarcomputes its own fan-out map to find the blocker holding up the most work. It passed onlystaleHighFanoutAgeThresholdMs, so core fell back toholdColumn: "todo"— a lane a renamed board doesn't have.overlapBlockedTodoCountstayed at zero, the high-fan-out threshold (>= 5) never tripped, and the warning simply never appeared while cards sat blocked behind one card.The flags were in scope the whole time: this component already receives
columnFlagsByTaskIdand hands it touseExecutorStats. It just never reached the fan-out.One type fix rides along, and it isn't cosmetic
ExecutorColumnFlagswas aPickthat droppedhumanReview, while its supplier inApp.tsxbuilds the map fromworkflow.columns.find(...).flags— the whole object. So the trait is present at runtime and only the type discarded it.That was harmless while these flags answered complete/archived/wip questions. It stops being harmless for the review role:
isReviewColumnRolereadsmergeBlocker || humanReview, so a lane hosting a human review without blocking merges would have classified as not-review. That's a wrong answer rather than a degradation — the "supplying the wrong flags" shape this program keeps re-finding — so thePicknow declares what the data already carries.I'd rather flag this than let it pass as a one-word type tweak: it's the kind of narrowing that reads as tidy and answers incorrectly.
Measured
ExecutorStatusBarsuite + both new fan-out suitestsccleanThe negative case is the one worth keeping: cards already in the renamed WIP lane must not count as held, or the fix trades a silent zero for a wrong number — which invites no scrutiny at all.
Still not done: TaskDetailModal
The third caller stays unfixed, and for a different reason than this one. It holds
detailColumnFlagsfor the open task only, while the fan-out needs a role answer for every dependent. So unlikeExecutorStatusBar, where the data was merely unthreaded, here it is genuinely absent — wiring it means deciding where per-task workflow metadata comes from in a modal, which is a design call rather than a mechanical repeat.That leaves the surface enumeration at 2 of 3 closed, stated rather than implied.