Skip to content

fix(dispatch): give every Grid Dispatch lane its own selector strip #859

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Juliusolsson05 merged 6 commits into from
Sep 11, 2026
Merged

fix(dispatch): give every Grid Dispatch lane its own selector strip #859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9d6fa2f
docs(dispatch): plan restoring the first Grid Dispatch lane's selecto…
Juliusolsson05 Sep 10, 2026
a93980f
fix(dispatch): give every Grid Dispatch lane its own selector strip
Juliusolsson05 Sep 10, 2026
0ef2b17
docs(dispatch): amend the first-lane strip plan for the row-scoped ⌥↓
Juliusolsson05 Sep 10, 2026
19dbccd
test(dispatch): pin each Grid Dispatch strip's lane, row binding and …
Juliusolsson05 Sep 11, 2026
318ad66
docs(dispatch): correct the first-lane strip regression history
Juliusolsson05 Sep 11, 2026
6ee33cc
fix(dispatch): withhold the empty-lane hint when the row offers no ag…
Juliusolsson05 Sep 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 144 additions & 0 deletions docs/superpowers/plans/2026-09-10-grid-dispatch-first-lane-strip.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
# Grid Dispatch First-Lane Strip Plan

**Issue:** #850 (regression from #691's side change; see the corrected history below)
**Branch:** `fix/grid-dispatch-first-lane-strip`, based on `origin/main` @ `48ea6912`

**Goal:** Every Grid Dispatch lane, including the first (leftmost) lane of each
row, gets its own compact index strip (`DispatchMiniList`), so swapping the
agent in any lane is one click on a selector that belongs to that lane.

## Why the first lane lost its selector

Before Grid Dispatch rows (parent of `524671b2`), the full index list beside the
lanes always wrote lane 0 (`setTiledLaneSession(0, …)`). The first lane skipping
the strip was correct then: the index *was* lane 0's selector, and a strip would
have been a second selector for the same lane.

`524671b2` (#687) gave every row its own index and retargeted that index to
`focusedLaneInRow ?? start`, meaning whichever lane of the row is focused. It
also gave **every** lane a strip, first lane included, for exactly that reason
("the index belongs to the row, not to its first lane").

`1eb9a472` (#691, a fix for waking hibernated agents placed into lanes) then
removed the first lane's strip as a side change ("Also: …"), arguing that the
row's index "IS its selector". That was already false, because the index
followed focus. From then on the first lane had no selector of its own.
Changing it takes two steps (click into the lane to move focus, then pick from
the index), while every other lane takes one click on its strip.

> **Corrected after review (2026-09-10).** The first version of this section
> said `524671b2` "kept the `column > 0` strip gate". Both PR reviewers found
> that wrong with `git show`, and I confirmed it: `524671b2` removed the gate
> and `1eb9a472` restored it. The fix is unchanged; the attribution matters
> because it tells the next investigator which change to read.

The fix keeps the index's follow-focus behavior, which the user chose on purpose,
and gives the first lane its strip back. It does not revert the index to lane-0-only.

## Changes

All under `src/renderer/src/workspace/dispatch/`.

1. **`TiledDispatchLayout.tsx`**
- Render `DispatchMiniList` for every lane: drop the `column > 0 &&` gate on
the strip. Keep the gate on `LaneBoundary`, since the first lane has no left
neighbour to resize against.
- Replace the "The row's FIRST lane has no strip" WHY comment with the reason
every lane has one: the index follows focus, so it cannot be any single
lane's dedicated selector.
- Empty-lane hint:
- Drop the `column === 0` "Pick an agent from the index" branch, because
every lane now has a strip.
- Delete the dead `gridRow.projectTabId` branch, so every row shows
"Pick an agent from the strip, or press ⌥↓ for the top of the index".

> **Amended during implementation (2026-09-10).** The first draft of this
> plan said to *fix* that read to `projectTabIds`, on the premise that ⌥↓
> takes the top of the global index and so "the top of the index" is false
> in a bound row. That premise is out of date. `524671b2` split the copy
> while ⌥↓ still walked the global list (`dispatchRows(workspace)` at that
> commit). `29ecd829` then made ⌥↓ walk the focused row's own filtered list
> (`tiledRowScopedRows` in `useKeybinds.ts`), which makes "the top of the
> index" true in a bound row too. The stale read happened to show that copy
> anyway, because `normalizeGridShape` folds `projectTabId` away. Fixing the
> read would have withheld a true promise, so the branch is deleted. Bound
> rows see no change. The hint is copy, so no test pins it; a WHY comment at
> the hint says not to re-add a binding branch while ⌥↓ is row-scoped.
2. **`DispatchMiniList.tsx`**
- Prop type `Pick<DispatchGridRow, 'projectTabId' | …>` becomes
`'projectTabIds'`. That is the field `rowScopedRows` actually reads. Runtime
behavior is already right because the full row object is passed; only the
declared contract is stale.

## Tests (`gridDispatchLayout.renderer.test.tsx`)

Written first, and each one must fail on `origin/main` before the fix:

- **Every lane has a strip.** A 2×2 grid renders 4 strips; a `[3, 1]` grid
renders 4 strips. These replace the two tests that pinned "first lane has no
strip".
- **The first lane's strip selects into the first lane, not the focused lane.**
With focus on lane 1 of row 0, clicking lane 0's strip calls
`selectTiledLaneSession(0, …)` and `setTiledFocusedLane(0)`. This is the
one-click navigation the issue is about.
The `DispatchMiniList` mock reports its lane's selection and forwards a click to
the layout's real `onSelect` closure, so the tests can tell lane 0's strip from
the others and observe which lane the layout writes. Also update the stale
"once each row's first lane lost its strip" comment on the row-targeting test.
That test's claim still holds.

(The first draft also listed a bound-row hint test. It was dropped with the
premise it relied on; see the amendment under Changes.)

## Out of scope

- Classic single-pane Dispatch (`DispatchLayout` without `tiled`). Its index is
still its only pane's selector.
- Index click semantics, keybindings, `DispatchAgentList`.

## Verification

On Node 24 (`.nvmrc`; Node 25 breaks happy-dom's `localStorage`):

- `NODE_ENV=test npx vitest run --project renderer src/renderer/src/workspace/dispatch/`
shows RED before the fix and GREEN after.
- `npm run typecheck` (vitest and electron-vite do not type-check).
- One full `npm test` before opening the PR.

The app is not launched. The layout contract is proven at the rendered-layout
level, which is where the #673 post-mortem says a layout fix has to be proven.

## Review (2026-09-10)

Two orchestrated reviewers ran, both on Codex: one on correctness and
behavior, one on tests, comments and conventions. A Claude reviewer was
attempted twice and never received its prompt (known issue #854: long
`create_agent` prompts fail for Claude children), so the second seat moved to
Codex. Verdicts: "Yes" and "With fixes". No Critical findings.

Resolved on this branch:

- **Wrong regression history** (both reviewers). Corrected in the layout's WHY
comment, this plan, the PR body and #850, as described above.
- **Strip-click test only exercised flat lane 0**, where `laneIndex`, `column`
and `start` are all 0. It is now a table over lanes 0, 2 and 3 with focus
elsewhere. Proven by mutation: `column` and `start` each fail it.
- **Strip `gridRow` wiring was untested.** New test: each strip receives its own
row's binding. Proven by mutation (`grid.rows[0]` fails it).
- **Expand test proved only that a handler was present.** It now asserts
`toggleDispatchRowExpandedParent(rowIndex, parentId)` per strip. Proven by
mutation (row hardcoded to 0 fails it).
- **Row-index comment overclaimed** what its case distinguishes. Reworded, and
a new test pins that the index fills the focused lane of its own row. That is
the premise of #850, and it is proven by mutation (a bare `start` fails it).
- **Ambiguous strip lookup.** `stripSelecting` asserts exactly one strip matches.
- **Empty-lane hint promised a pick in a row that offers no agents** (a
pre-existing issue in the code this PR rewrote). The hint is now gated on
`rowScopedRows(rows, gridRow)` containing an agent, the same filter the strip
and ⌥↓ use. The test was RED before the change.

Not changed, and recorded as a known limitation: the fixed 46px strip combined
with the percentage-only `LANE_MIN_FRACTION` (0.08 of the lane region) can leave
a lane dragged to its minimum with no visible content. That already applied to
every other lane; this PR extends it to the first lane. The fix is a
minimum-content-width rule for lanes and belongs in its own change.
21 changes: 15 additions & 6 deletions src/renderer/src/workspace/dispatch/DispatchMiniList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,17 @@ import {
dispatchRowTitle,
} from '@renderer/workspace/dispatch/DispatchAgentList'

// The Tiled Dispatch lane selector, deliberately stripped to NOTHING but the
// The Grid Dispatch lane selector, deliberately stripped to NOTHING but the
// index chips ([A1], [A2], ★1 …) — no titles, no activity dots, no badges,
// no section headers, no tab separators. Rationale (user's call): the rich
// context already lives one lane over in the full pinned index; repeating any
// of it here just wastes the narrow column. To identify what a chip refers
// to, glance back at the index (or hover the chip for its tooltip).
// context already lives in the row's full index at the row's left edge;
// repeating any of it here just wastes the narrow column. To identify what a
// chip refers to, glance back at that index (or hover the chip for its
// tooltip).
//
// Every lane carries one, the row's first lane included (#850). The full index
// fills whichever lane of its row is focused, so the strip is the only
// selector addressed to one specific lane.
//
// Each chip is visually identical to the full list's index cell — same width
// and same activity background — because it's painted with the SAME
Expand All @@ -28,8 +33,12 @@ import {
type Props = {
rows: DispatchAgentRow[]
/** The grid row this strip belongs to — supplies its project binding and
* child density, so two strips over the same workspace legitimately differ. */
gridRow?: Pick<DispatchGridRow, 'projectTabId' | 'capChildren' | 'expandedParents'>
* child density, so two strips over the same workspace legitimately differ.
* `projectTabIds`, not the legacy `projectTabId`: that is the field
* rowScopedRows filters on, and normalizeGridShape folds the legacy one
* away on read. Naming the legacy field here type-checked (every key is
* optional) while promising a binding the strip never read. */
gridRow?: Pick<DispatchGridRow, 'projectTabIds' | 'capChildren' | 'expandedParents'>
selectedSessionId?: SessionId
focused: boolean
onSelect: (row: DispatchAgentRow) => void
Expand Down
114 changes: 78 additions & 36 deletions src/renderer/src/workspace/dispatch/TiledDispatchLayout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
DispatchEmpty,
} from '@renderer/workspace/dispatch/DispatchAgentList'
import { DispatchMiniList } from '@renderer/workspace/dispatch/DispatchMiniList'
import { rowScopedRows } from '@renderer/workspace/dispatch/rowScopedRows'
import type { DispatchGridRow, SessionId, TabId } from '@renderer/workspace/types'

type Props = {
Expand Down Expand Up @@ -254,6 +255,16 @@ function GridRowView({
const indexFraction = clampIndexFraction(gridRow.indexFraction ?? DEFAULT_INDEX_FRACTION)
const focusedLaneInRow =
grid.focusedLane >= start && grid.focusedLane < end ? grid.focusedLane : null
// Whether this row's strips and ⌥↓ have anything to pick from. It asks the
// SAME filter both of them use (rowScopedRows: project binding + child cap;
// ⌥↓ reaches it through tiledRowScopedRows in useKeybinds), so the empty-lane
// hint below can never promise a pick the row does not actually offer. A
// hand-rolled "any row in a bound tab" check here would be a second
// definition of "what this row offers", and would drift from the first.
const rowOffersAgents = useMemo(
() => rowScopedRows(rows, gridRow).some(item => item.kind === 'agent'),
[rows, gridRow],
)

const rowRef = useRef<HTMLDivElement | null>(null)
const laneRegionRef = useRef<HTMLDivElement | null>(null)
Expand Down Expand Up @@ -358,33 +369,52 @@ function GridRowView({
workspace={workspace}
/>
)}
{/* The row's FIRST lane has no strip: the row's own index list
sits directly beside it and is its selector. That pairing is
the point of giving every row its own index — a strip there
would be a second selector for the same lane, six inches from
the first, and it costs 46px of the widest lane in the row.
Every OTHER lane needs its own, because the index is already
spoken for. */}
{column > 0 && (
<div className="flex-shrink-0 min-h-0">
<DispatchMiniList
rows={rows}
gridRow={gridRow}
selectedSessionId={lane?.selectedSessionId}
focused={focused}
onSelect={row => {
void workspace.selectTiledLaneSession(laneIndex, row.sessionId)
workspace.setTiledFocusedLane(laneIndex)
}}
// Without this the strip's "+N more" renders as a button and
// does nothing — an affordance that promises an action it
// cannot perform, in the exact case the cap exists for.
onToggleExpandedParent={sessionId =>
workspace.toggleDispatchRowExpandedParent(rowIndex, sessionId)
}
/>
</div>
)}
{/* EVERY lane gets its own strip, the row's first lane included
(#850). The row's index fills whichever lane of the row is
FOCUSED (selectIntoRow above), so it is no single lane's
selector.

History, because this rule has flipped twice. Before Grid
Dispatch, one sidebar index always wrote lane 0, so lane 0
correctly had no strip. #687 (524671b2) made the index
per-row and follow focus, and gave every lane a strip for
exactly that reason. #691 (1eb9a472, a lane-wake fix) then
removed the first lane's strip as a side change, arguing that
the index "IS its selector". That was already false, because
the index followed focus, and it left the first lane as the
only one that took two gestures to change (click into it, then
pick from the index). Re-removing this strip is only right if
the index stops following focus; the renderer test "fills the
focused lane from a row's index…" fails first if it does.

Single-lane rows get a strip too, although their index can
only ever fill that one lane. A strip that appeared only when
a second lane was added would shove the existing lane's content
46px sideways at the moment the user is rearranging the row.
One rule for every lane also keeps the chip column in the same
place in every lane, which is what makes it scannable. */}
<div className="flex-shrink-0 min-h-0">
<DispatchMiniList
rows={rows}
gridRow={gridRow}
selectedSessionId={lane?.selectedSessionId}
focused={focused}
// This lane's index, never `grid.focusedLane`: the strip is
// the lane-addressed selector, and the index is the one that
// follows focus. If both followed focus, the first lane would
// lose its one-click selector again (#850).
onSelect={row => {
void workspace.selectTiledLaneSession(laneIndex, row.sessionId)
workspace.setTiledFocusedLane(laneIndex)
}}
// Without this the strip's "+N more" renders as a button and
// does nothing — an affordance that promises an action it
// cannot perform, in the exact case the cap exists for.
onToggleExpandedParent={sessionId =>
workspace.toggleDispatchRowExpandedParent(rowIndex, sessionId)
}
/>
</div>
<div
className="relative flex-1 min-w-0 min-h-0"
onMouseDownCapture={() => {
Expand Down Expand Up @@ -412,16 +442,28 @@ function GridRowView({
// Advertising it in an unfocused lane would tell the user
// to press something that yanks the agent they are working
// with and leaves this lane untouched.
// The FIRST lane of a row has no strip (its index list is
// its selector), so naming one there points the user at
// something that is not on screen.
//
// One copy for every lane. Every lane has a strip (#850),
// and ⌥↓ walks the focused ROW's own filtered list
// (tiledRowScopedRows in useKeybinds, since 29ecd829), so
// "the top of the index" is true in a project-bound row too.
// An older branch withheld that phrase from bound rows
// because ⌥↓ used to walk the global list. It keyed off the
// legacy `projectTabId`, which normalizeGridShape folds into
// `projectTabIds` on read, so it was already dead and showed
// this copy anyway. Do not re-add a binding branch unless ⌥↓
// stops being row-scoped: "fixing" that read to
// `projectTabIds` would withhold a promise that is true.
//
// And none at all when the row offers no agents (a row
// bound to projects that currently have none). Its strip is
// empty and ⌥↓ returns without doing anything, so either
// half of the hint would promise a pick that cannot happen.
// The bare "Empty lane" stays; the row's index is where the
// user sees why.
hint={
focused && !lane?.selectedSessionId
? column === 0
? 'Pick an agent from the index, or press ⌥↓'
: gridRow.projectTabId
? 'Pick an agent from the strip, or press ⌥↓'
: 'Pick an agent from the strip, or press ⌥↓ for the top of the index'
focused && !lane?.selectedSessionId && rowOffersAgents
? 'Pick an agent from the strip, or press ⌥↓ for the top of the index'
: undefined
}
/>
Expand Down
Loading