Skip to content

feat(right-panel): comment mode in the Code tab, copy-to-clipboard flush#879

Closed
srid wants to merge 45 commits into
masterfrom
juicy-gum
Closed

feat(right-panel): comment mode in the Code tab, copy-to-clipboard flush#879
srid wants to merge 45 commits into
masterfrom
juicy-gum

Conversation

@srid
Copy link
Copy Markdown
Member

@srid srid commented May 13, 2026

Kolu's Code tab now has a comment mode: a 💬 toggle reveals a tray, line-selection in any file viewer attaches a free-text note to that path:line[-end], comments accumulate across every file you visit in the worktree, and "Copy to clipboard" serializes the queue and clears the tray. Paste the result into whichever agent terminal makes sense — claude, codex, opencode, plain shell, or a GitHub comment — Kolu doesn't route anything.

The motivating problem: agents already write review-worthy artifacts (plans as Markdown, code, configs) and the only way to send pinpoint feedback back today is to re-type "in foo.md, the goals section — tighten it" by hand. The Code tab already shows every file in the repo; this PR adds the missing affordance to point at parts of those files, queue notes, and hand the batch off.

Closes #878.

How it works

┌── Code tab toolbar ───────────────────────────────────┐
│  [mode chips]  [file search]  [💬 Comment]            │
├── file tree ──────────────────────────────────────────┤
│  ▸ docs/                                              │
│    ▸ spec.md                                          │
│  ▸ src/                                               │
│    ▸ foo.ts                                           │
├── file viewer (Pierre FileDiff / FileView) ──────────┤
│  12  ┃ alpha                                          │
│  13  ┃ beta             ← click gutter to anchor      │
│  14  ┃ gamma                                          │
├── comments tray ─────────────────────────────────────┤
│  docs/spec.md:L12-18  > tighten the goals       [×]   │
│  src/foo.ts:L42       > extract this helper     [×]   │
│  [composer ──────────────────────────────────────]    │
│  [target chip] [textarea] [Add comment]               │
│                                       [Copy & clear]  │
└───────────────────────────────────────────────────────┘

The Pierre line-selection that already powered the right-click Copy path:N menu now drives the comments composer too — same controller (useLineSelection), same shadow-DOM selectors. The diff view and browse view both wrap a single CodeMenuFrame at the same depth, so any text file (.md, .ts, .css, configs, whatever) accepts line-anchored comments without per-kind logic.

Behaviors

  • Pinpoint anchor — click a single gutter line or drag a range; the composer's target chip shows path:Lstart-end
  • Cross-file accumulation — the tray collects entries from every file you visit; sorted by (path, startLine) so the paste reads as a repo walk
  • Markdown clipboard payload — flush emits [kolu comments v1] envelope + a Markdown bullet list with code-spanned GitHub-style refs, so the paste renders cleanly in any agent terminal, GitHub issue, or chat surface
  • Copy-and-clear — destructive by design: the tray empties once the payload is on the clipboard, so the next review starts fresh
  • Per-worktree persistencemakePersisted keyed by git.repoRoot; reload doesn't lose the in-progress queue, and switching the active terminal between worktrees shows each worktree's own tray
  • Cmd/Ctrl + Enter submits the composer; plain Enter is a newline for multi-line notes

Clipboard payload

[kolu comments v1]

- `docs/spec.md:L12-18` — tighten the goals section
- `src/foo.ts:L42` — extract this helper

Why no CLI / no oRPC / no server delivery

Earlier sketches had Kolu paste feedback directly into the active agent terminal via terminal.sendInput. The clipboard is universal — it works for every agent CLI, plain shells, and paste targets outside Kolu (issues, chat) — and keeps the user in control of when and where to deliver. Zero new server code, zero contract surface.

Review trail

The 24 commits on this branch are a deliberate progression: one feature commit, then hickey + lowy structural reviews and /code-police (rules → fact-check → elegance via /simplify) each landing one commit per finding. Reading git log --reverse should be a coherent design walkthrough.

Use 💬 Comment in the Code-tab toolbar after the agent writes you a Markdown plan, code, config, anything. Paste the flushed block back into the agent's prompt — it's just text, no special parsing required.

Try it locally

```sh
nix run github:juspay/kolu/juicy-gum
```

Generated by `/do` on Claude Code (model `claude-opus-4-7`).

srid added 22 commits May 13, 2026 10:14
Adds a 💬 toggle in the Code-tab toolbar that reveals a comments tray
at the bottom of the panel. Pierre's existing line-selection drives a
composer that attaches a free-text note to a (path, line-range) target.
Comments accumulate across every file visited in the worktree and flush
to the system clipboard as a versioned text block — "Copy to clipboard"
is destructive by design: the tray auto-clears once the payload has
been written.

The in-progress queue is persisted via `@solid-primitives/storage`'s
`makePersisted`, keyed by `git.repoRoot`, so each worktree has its own
tray and an accidental reload doesn't lose work.

No new oRPC namespace, no server-side delivery, no `kolu artifact` CLI
— pasting into an agent terminal is the universal hand-off mechanism
and keeps the user in control of when/where feedback lands.

Closes #878
The trailing-underscore export only existed to serve a single `false`
call site in CodeTab. Renaming to disableCommentMode + dropping the
boolean argument makes the comment-mode API symmetric with
toggleCommentMode and removes the smell.
useComments.ts braided two independently-volatile concerns: a global
session-wide UI toggle and per-repoRoot comment buckets. They share no
state or lifecycle. Moving the toggle to its own module unfuses the
change axes — a future per-pane toggle or backend swap touches one
file instead of two.
The module-level `buckets` Map grew on every distinct repoRoot
encountered and never shrank, so a long session that walked many
worktrees would accumulate dead entries. Add a createEffect/onCleanup
sweep: when a consumer moves away from a repoRoot whose bucket is
empty, delete the in-memory entry. Persisted localStorage data stays
put so a return visit still rehydrates.
useComments writes the bucket to localStorage as a bare `Comment[]`
today. A future field rename would silently corrupt existing buckets
because there is no schema marker to dispatch on. Wrap the persisted
form as `{ v: 1, comments }` and tolerate the legacy bare-array
shape on read — writes always emit the envelope, so future bumps to
`v: 2` get an explicit migration step instead of guessing.

Mirrors the clipboard payload's existing `[kolu comments v1]` header
discipline at the persistence layer.
CodeMenuFrame previously forwarded selection changes via a deferred
createEffect. After a keyed file switch (\`<Show keyed>\` in CodeTab
remounts CodeMenuFrame), the new instance's first frame would still
carry the previous file's range until the effect propagated null —
long enough for a fast Ctrl+Enter to submit a comment anchored to
the wrong file.

Push the forwarder one level deeper into useLineSelection as a
synchronous onChange callback fired inside the same call that
mutates the internal range. Drop the createEffect from CodeMenuFrame.
Same single-point-of-wiring property; no stale-frame window.
BrowseFileView previously wrapped its own CodeMenuFrame and forwarded
onSelectionChange as a pure pass-through prop — carrying a
comments-tray concern through a viewer that knew nothing about
comments. The diff path wraps CodeMenuFrame at the outer (CodeTab)
level; the browse path now does the same. BrowseFileView accepts the
LineSelection controller in via props and wires it to FileView
directly.

Both viewer paths now sit at the same depth under a single
CodeMenuFrame at the CodeTab layer — symmetric, no prop-drill,
selection forwarding stays a property of the frame.
The tray's visibility predicate is `commentMode OR comments.length > 0`.
With queued comments, clicking the close ✕ would call
disableCommentMode but the OR's second arm would keep the tray open —
the click silently no-oped. Mark the button `disabled` in that state
and surface a tooltip telling the user to clear or copy first.
The draft signal was lifted into CodeTab purely to survive tray
remounts. With the tray always-rendered while comment-mode is on AND
the only remount source (mode-off + comments-empty) being a discard
gesture anyway, the lift no longer earns its keep. Inline as
createSignal in CommentsTray. CodeTab stops carrying tray-local state
through props.
The Hickey-E commit (a4f119b) introduced setAndForward but the
`createSignal(initialRange())` + `{ defer: true }` effect still
skipped the initial value through onChange. After <Show keyed>
remounted CodeMenuFrame on file switch, the parent's currentRange
signal retained the previous file's range; the composer's target
chip and canAdd() both pointed at the wrong file until the user
made a new selection.

Drop the defer flag and seed the signal from inside the effect
(setAndForward), so the file-switch reset flows through onChange
in the same frame the file changes.
…cket

JSON.parse failures and unrecognized shapes were returning `[]`
silently — the user couldn't tell a fresh worktree apart from a
corrupted persistence bucket. Both paths now toast an error with the
repoRoot and (where available) the parse error message, so the
failure is visible at the user surface and points at the
localStorage key that's wrong.
The clipboard write happens inside the React-style render handler
triggered by a button click; the read can outrun it on slower
runners (aarch64-darwin) and pass on x86_64-linux. Switch from
`page.evaluate(...) + sync assert` to `page.waitForFunction(...)`
with POLL_TIMEOUT so the read polls until the expected condition
holds or the timeout fires.
The two Then handlers for "should list {int} comment(s)" had
byte-identical function bodies, only the step phrase singular/
plural differed. Extract a single helper so the two handlers can't
drift; the singular/plural split survives so feature files read
naturally.
…context fallback

The hand-rolled `navigator.clipboard.writeText` call rejected on
plain-http (any non-localhost host). The shared helper at
terminal/clipboard.ts:21 already wraps writeText with a hidden-
textarea + document.execCommand("copy") fallback that survives that
case. Keep the local try/catch so the tray only clears on
successful write.
…mmentSerialize

formatLineRef and formatLineRange both branched on `start === end`
to pick "N" vs "N-M". Extract formatRange(start, end) in
ui/lineRef.ts so each prefix style (`path:`, `L`, future `#L`)
adds only its prefix character around one shared helper.
The hand-rolled mintId (process-local counter + base36 timestamp)
duplicated what crypto.randomUUID gives for free, with weaker
collision properties (counter resets to 0 on page reload — two
comments minted in the same millisecond across a reload could
collide if Date.now matches, since counter would also restart).
Use the platform helper.
…Picker chip

The 💬 emoji + "On/Off" text broke the SVG-icon convention (every
other right-panel toolbar control draws from ui/Icons.tsx) and the
hand-rolled border styling diverged from the chip pattern used by
the adjacent ModeChipPicker. Add CommentIcon to the registry and
copy the ModeChipPicker chip class so the comment toggle sits
visually with the mode chips it lives next to. aria-pressed +
data-active drive the active state — no "On/Off" label needed.
Two near-identical button class strings ("Copy to clipboard" and
"Add comment") are now one TRAY_PRIMARY_BUTTON_CLASS constant.
Eight `data-testid` strings flow from a single commentsTestIds
object exported from CommentsTray.tsx — markup now can't typo a
testid without a TypeScript error.

The test-side selectors stay as string literals (the tests
package doesn't import from client) and document the contract in
a comment.
addComment, removeComment, and clear each repeated
`const r = repoRoot(); if (!r) return; const b = bucket(r);`.
Factor into a withBucket(fn) helper so the three methods become
one-liners.
The tray's render fanned out six separate `props.api.comments()`
reads (header count, copy-disabled, close-disabled, close-title,
list-fallback gate, For each). Each call walked the bucket Map +
the Solid signal, and each established its own reactive
subscription. One createMemo at the top shares one tracked read.
Project convention is to write comments only when the WHY is
non-obvious. Several block comments in this PR narrated the diff or
restated what well-named identifiers already convey; collapse those
to one or two lines each, keeping the genuinely non-obvious WHYs
(file-switch reset semantics, OR-arm gate on the close button,
versioned persisted shape rationale).
Pierre's VirtualizedFile measures its viewport at mount; mounting
into the comment-mode-shrunk viewport pushed line 2 out of the
initial render range, so a [data-column-number="2"] locator timed
out. Open the file first, then toggle comment mode — Pierre keeps
the already-rendered rows. Line 1 still works regardless because
it's always at the top of the buffer.

Also drop the redundant second toggle-inspector after reload — the
right panel state is server-persisted and restores itself.
@srid
Copy link
Copy Markdown
Member Author

srid commented May 13, 2026

Hickey/Lowy Analysis

# Lens Finding Disposition
1 Hickey setCommentMode_ raw-setter export is asymmetric with toggleCommentMode (commit 73897198) Fixed in this PR
2 Hickey useComments.ts braided global comment-mode toggle with per-repoRoot bucket persistence (3a7b5cb7) Fixed in this PR
3 Hickey Module-level buckets Map grew without cleanup across repoRoot switches (7728360d) Fixed in this PR
4 Hickey currentRange forwarding via deferred createEffect had a stale-frame window across file switches (a4f119bf) Fixed in this PR
5 Hickey Close button silently no-oped while comments queued (the OR-arm of trayVisible kept the tray open) (03bdabae) Fixed in this PR
6 Lowy Persisted Comment[] shape was unversioned; future field renames would silently corrupt buckets (61742351) Fixed in this PR
7 Lowy BrowseFileView pass-through of onSelectionChange carried a comments-tray concern through a viewer that knew nothing about comments (87ac8270) Fixed in this PR
8 Lowy Composer draft was lifted into CodeTab and prop-drilled into the tray (4f497b06) Fixed in this PR
9 Lowy Clipboard payload format encapsulation, repoRoot keying seam, composer/tray separation No-op (correctly bounded already)

Hickey rationale

The diff carried two layers of structural tension that the post-implement review surfaced. The bigger one was a deferred effect projecting per-file selection state out across a component boundary (useLineSelection.rangecreateEffect({defer: true})currentRange in CodeTab → tray props). After <Show keyed> remounted CodeMenuFrame on file switch, the new instance's first frame retained the previous file's range; a fast Ctrl+Enter on the composer could submit a comment anchored to the wrong file. The fix pushed forwarding one level deeper into useLineSelection itself as a synchronous onChange callback — the file-switch reset now reaches external observers in the same frame the file changes.

The smaller findings: a leaky raw setCommentMode_(boolean) export that only served a single false call (renamed to disableCommentMode()), a buckets Map that grew unbounded across long sessions that touched many worktrees (now swept via onCleanup/createEffect when buckets become empty), a global comment-mode boolean braided into useComments.ts alongside per-worktree state (split into useCommentMode.ts), and a close-button that silently no-oped when comments were queued because the OR-arm of trayVisible kept the tray visible regardless of the toggle (now disabled with a title explaining why).

Lowy rationale

The volatility decomposition came back narrow but pointed. Three axes are well-bounded already — clipboard payload format is fully encapsulated in commentSerialize.ts, identity keying through useComments(repoRoot: Accessor<string | null>) lets a caller substitute terminal id or any other string accessor without signature change, and composer + tray-list share one volatility axis (both change when comment UX evolves). The three that needed work:

  1. The persisted bucket shape was unversioned at the persistence layer even though the clipboard payload carries [kolu comments v1]. A future field rename would have silently corrupted buckets. Wrapping the persisted form as { v: 1, comments: Comment[] } with a tolerant legacy reader puts the persisted shape on its own version axis, mirroring what the clipboard format already does.

  2. BrowseFileView.onSelectionChange was a pure pass-through. The diff path wrapped CodeMenuFrame at the CodeTab layer; the browse path tucked it inside BrowseFileView, forcing a comments-tray-shaped prop through a viewer that knew nothing about comments. Hoisting CodeMenuFrame to the same depth in both paths makes BrowseFileView accept a selection: LineSelection directly — symmetric, no prop-drill, the comments concern stays at the CodeTab layer where it belongs.

  3. draft was lifted into CodeTab to survive tray remounts. With the close button now disabled while comments are queued, the only remount source is the empty-tray-mode-off transition — a discard gesture by definition. The lift no longer earned its keep; moved back to a local createSignal in CommentsTray.

srid added 2 commits May 13, 2026 11:24
The docs step previously only synced README.md + packages/surface/
README.md, leaving the hand-maintained features array on the kolu.dev
landing page free to drift behind the project's own description.
Adding website/src/pages/index.astro (and the `NN features` count
chip) here means future /do runs catch the gap at the docs step.
Mirrors the new "### Comments" section in the top-level README so
kolu.dev's features list stays in sync with the project's own
description. Bumps the "NN features" count chip to 09.
srid added 2 commits May 13, 2026 11:35
… refs

The clipboard now emits a Markdown bullet list with code-spanned
GitHub-permalink-flavored line refs (`path:L42` single, `path:L12-18`
range) instead of two-space-separated `path  L<range>` blocks. Same
`[kolu comments v1]` envelope so existing agent-side parsers can
detect and dispatch on the version, but the body now renders
natively when pasted into agent terminals, GitHub issues, or Slack.

Adds `formatLPathRef` to ui/lineRef.ts alongside `formatLineRef`
(the L-less VS Code / Vim flavor used by the right-click "Copy
path:N" menu). Tray-list and target-chip display also use the new
unified format so what the user sees in the tray matches what the
paste contains, byte-for-byte.

Example payload:
  [kolu comments v1]

  - `docs/spec.md:L12-18` — tighten the goals section
  - `src/foo.ts:L42` — extract this helper
srid added 7 commits May 13, 2026 12:47
… tip

Three discoverability surfaces in one wiring:
- Cmd+Shift+/ (Mod+Shift+?) toggles comment mode globally; falls
  through to xterm thanks to the ACTIONS registry's matchesAnyShortcut
- "Toggle comment mode" lands in the Cmd+K palette via
  actionPaletteCommand — same wiring as toggleRightPanel
- Ambient tip surfaces the keybind on startup so first-time users
  discover the feature without reading the toolbar

The handler also expands the right panel and switches the Code tab to
browse mode before flipping the toggle — the keybind/palette flow
would be confusing if mode toggled on while the tray sits behind a
collapsed inspector panel.
The third discoverability surface — line-gutter right-click now offers
"Add comment on path:Lrange" alongside the existing "Copy path" and
"Copy path:N" entries. Clicking it enables comment mode (no-op if
already on) and focuses the composer textarea, so a user who selected
a line and right-clicked is one keystroke from typing a note.

Type-level changes:
- CodeContextMenuItem becomes a discriminated union of `textToCopy`
  and `onClick` variants; CodeContextMenu's handleItem dispatches on
  which field is present.
- useLineSelection gains an `onAddComment` option; buildItems emits
  the menu entry only when both a range is selected AND the parent
  wired the callback. CodeMenuFrame forwards conditionally so call
  sites that don't want the entry (none today, but the surface is
  honest about it) don't have to nullify a stale handler.

The existing context-menu assertion in code-tab.feature is widened to
expect the third entry. All 39 code-tab scenarios + 3 comments
scenarios pass.
Fourth discoverability surface — when the active terminal's worktree
has queued comments, an accent-tinted `💬 N` badge appears in the
chrome bar's control cluster (between RecordButton and the inspector
toggle). Click opens the right panel + Code tab; the tray is already
visible whenever comments.length > 0, so no extra scroll-to wiring
is needed.

The badge `<Show when={count() > 0}>` gates rendering on the count,
so users not actively reviewing don't see it. Reads the active
terminal's repoRoot reactively so cross-worktree terminal switches
update the badge instantly.
Adds a top bullet covering the toolbar chip, Cmd+Shift+/ keybind,
command palette entry, right-click "Add comment on path:Lrange"
context-menu item, and the chrome-bar `💬 N` count badge — so the
README's description of the feature matches what's actually wired
into the UI.
…ayload

The clipboard now emits a bare Markdown bullet list. Users (or agent
prompt templates) own whatever framing wraps it — "Apply these
comments:", "## Review notes", a code-fence, nothing at all. The
envelope was solving a problem we didn't have: agents reading pasted
text don't dispatch on a header, they just consume the structure that
follows. Removing it makes the paste look natural in any host
(agent terminal, GitHub issue, chat) without a Kolu-shaped wrapper.

The persisted localStorage shape keeps its `{ v: 1, comments }`
envelope — that's a separate concern (forward-compat for stored
data) and isn't user-visible.

Drops the e2e "kolu-comments-v1 envelope" step + the matching
assertion in comments.feature.
…ump from tray

Click a line in comment mode and a tiny composer pops up anchored next
to that line (GitHub-PR style) — no mouse trip to a bottom textbox.
Enter submits (Shift+Enter inserts a newline; Cmd+Enter stays bound to
"New terminal" globally). Esc dismisses.

The tray is now read-only: click a line ref to jump, pencil to edit
(re-opens the popover prefilled), trash to delete one. The tray header
paints with an accent fill whenever the queue is non-empty so it's
hard to accidentally ignore.

Files with queued comments get a `●` badge in the Code-tab file tree
via Pierre's `renderRowDecoration` — discoverable from the top of the
tree, not just the bottom tray. solid-pierre's FileTree wrapper now
forwards the prop and nudges Pierre to re-render decorations on change
by piggybacking on `setGitStatus` (the only public mutator that
triggers a row re-render).

The popover anchors to Pierre's `[data-selected-line]` element via
`getBoundingClientRect` and re-measures on scroll/resize. Pierre commits
its selection DOM via requestAnimationFrame, so the first measure right
after `setEditTarget` races that frame — we retry across a handful of
frames and fall back to a viewport corner so the popover is always
visible at minimum.

CommentComposer is its own component so the same widget services both
new-comment and edit-existing flows. Tray-driven edit pushes the
comment's file + range through the same `selectedRange` pipeline the
terminal `path:line` click uses, then opens the popover in edit mode at
the (now-selected) line.

`updateComment(id, text)` added to useComments so edit actually saves.
E2e covers add, copy, edit-via-pencil, and reload-survival.
@srid srid mentioned this pull request May 13, 2026
srid added 12 commits May 13, 2026 14:29
… lookup

Pierre's diff renderer attaches an open shadow root to its
`<diffs-container>` custom element (verified in
`@pierre/diffs/components/web-components.js:attachShadow({ mode: "open" })`),
so a plain `viewerEl.querySelector("[data-selected-line]")` couldn't
reach Pierre's selected-line marker in diff mode and always returned
null — the popover then fell back to the viewer corner instead of
anchoring at the line. Browse mode worked because FileView renders in
light DOM.

Replace the lookup with `deepQuerySelector` that walks every
descendant's open shadow root, so the same anchor logic works for
browse and diff alike.

Drop the page-corner fallback: anchoring at (16, 80) was useless once
the proper anchor failed. Track the last `pointerdown` inside the
viewer and use those coords as the fallback — the menu portal renders
outside `viewerEl` so right-click → "Add comment" doesn't poison the
coords, and a left-click commit that races Pierre's selection rAF
still lands the popover under the user's cursor instead of in another
country.
Agent CLIs (`claude`, `codex`, `opencode`) parse `path:N` natively for
their `Read` tool — that's `grep -n` style, what ripgrep / vim /
VS Code / stack traces all emit. The `path:LN` flavor is GitHub-
permalink URL syntax and gains nothing in a paste-to-terminal flow:
agents don't have a special parser for it, and a human reading the
bullet list doesn't need the `L` to recognize a line number.

Flip serializeComments, the composer's anchor chip, and the tray's
line-ref display from `formatLPathRef` to `formatLineRef`. The
"Copy file:LN" right-click menu entry stays — that's a user-chosen
GitHub-permalink alias, separate from the clipboard-payload concern.
…click to compose

Three problems the previous PR shipped with:

1. **Popover appeared below the line, blocking context.** Pierre's
   selected `<code>` element fills the full grid row, so its
   `getBoundingClientRect().right` was the viewer edge — useless for
   "right of the line". Use a DOM `Range` over the line content for a
   tight box around just the rendered text, then anchor the popover
   to `textRect.right + 12`. Clamp to viewport so a long line doesn't
   push the popover past the right edge.

2. **Popover orphaned over the canvas after switching terminal /
   worktree / right-panel tab.** CodeTab stays mounted across these
   transitions (#818), and the popover lives in a Portal mounted to
   `<body>`, so it kept rendering even when the viewer wasn't visible.
   Three guards: `measure()` bails when the viewer's rect is zero
   (panel hidden via `display:none` from the parent tab switcher),
   `createEffect` watching `rightPanel.activeTab()` clears
   compose state on tab switch, `createEffect` on `repoRoot` clears
   it on terminal switch.

3. **Click-line auto-opened the composer with no discoverable
   affordance for existing comments.** Replace the auto-popover with
   a "+" bubble pinned to the right of the selected line — the
   discoverable click target. Mount `LineCommentMarker` per queued
   comment in the currently-shown file (`💬` glyph anchored at
   `[data-line-index="<startLine-1>"]` inside Pierre's shadow root).
   Both bubbles open `CommentComposer` on click — new vs edit.
   Right-click "Add comment" and the tray pencil still bypass the
   bubble (the user already made an explicit choice through those).

`deepQuerySelector` lives in `LineCommentMarker.tsx` and is re-used
by `InlineCommentPopover.tsx` — one shadow-DOM-aware lookup, one
positioning math, both surfaces share it.

E2e covers the new bubble step (60 steps, +6).
Three fixes from user-reported live testing:

1. **+ bubble persisted after submit** instead of becoming 💬. The
   "+" key check didn't consult `commentsApi.comments()`, so the
   bubble re-mounted on the same line even after the comment was
   added. Encode "no comment exists at the selected range" into the
   key — when a comment exists, "+" returns null and the For-loop's
   "💬" bubble takes over (mutually exclusive at the same line).

2. **+ bubble orphaned over canvas after tab switch / mid-transition.**
   `lineRect` reads were correct but `Range.getBoundingClientRect`
   returns zero-width for Pierre's tokenized inline content (each
   syntax token is a separate span; the Range collapses
   unpredictably across browsers), causing `textRect.width === 0`
   and the fallback to anchor at the line's LEFT edge instead of
   right. Iterate the actual child rects and take the rightmost
   edge — robust for grid-rendered diff lines AND plain file-view
   lines.

3. **Bubbles/popover didn't track Pierre's transform-based scroll.**
   The virtualizer uses `transform: translateY` for scroll, which
   doesn't fire `scroll` events on the document. Replace the
   one-shot measure + scroll-listener with a continuous rAF loop
   while the marker/popover is open — one rect read per frame is
   cheap and catches transform-driven reflows.

Plus belt-and-suspenders: every orphan condition (no repo, panel
inspector, no file, panel collapsed) is encoded into the marker's
`key` accessor so the bubble disappears reactively the instant any
guard flips, regardless of which effect runs first. Tab switch no
longer clears `currentRange` so returning to the Code tab restores
the "+" without re-clicking — only `editTarget` clears (user
changed context).

Also hide markers when the line scrolled out of the viewer (above
OR below) — they were floating over the toolbar / tray when their
anchor line scrolled past.

E2e: 4 new scenarios cover the edge cases the user called out
(8 scenarios / 118 steps total):
- + becomes 💬 once the line carries a queued comment
- 💬 stays visible after disabling comment mode (discovery)
- 💬 click opens the composer in edit mode, prefilled
- bubbles disappear when switching to inspector tab, reappear on return
Pierre stores two attributes per line:
- `data-line="N"` — the **file line number** (1-based, the user's actual
  line number)
- `data-line-index="N"` — Pierre's internal **render-position index**
  (0-based, skips diff context lines)

The previous selector `[data-line-index="${startLine - 1}"]` worked in
browse mode only by coincidence (first few file lines line up with
render indices), and never in diff mode where the rendered output
skips most context lines — file line 46 of a 200-line file might be at
render-index 8 in a Local diff, so the selector returned null and the
"💬" bubble never appeared.

Use `[data-line="${startLine}"]` — Pierre sets this from
`lineInfo.lineNumber` in `processLine.js`, which is the actual file
line, uniform across the browse / local / branch render modes.

The "+" bubble uses `[data-selected-line]` (set by Pierre on the
selected content element regardless of mode), so this fix is
strictly about the existing-comment-bubble side.
…ranch

Two non-uniform spots — user-reported "the bubble only appears in all
files view, not in branch / local":

1. **FileDiff wrapper didn't expose `setSelectedLines`** so the
   diff-mode CodeMenuFrame couldn't push Pierre's selection from the
   tray jump / pencil flows. The popover's `[data-selected-line]`
   lookup then returned null in diff mode and clicking 💬 silently
   did nothing. Pierre's `FileDiff` class supports `setSelectedLines`
   (`@pierre/diffs/dist/components/FileDiff.js:169`); the wrapper now
   forwards a `selectedLines` prop through a defer-tracked effect,
   matching how `FileView` already worked.

2. **`handleTrayJumpTo` / `handleTrayEdit` force-flipped to "browse"**
   on entry. This was a bandaid for #1 — with FileDiff now honoring
   `selectedLines`, the flip is unnecessary noise; users keep their
   chosen view.

E2e coverage expanded from 8 → 13 scenarios (118 → 200 steps),
covering the bubble flow uniformly:

- "+ bubble flow works in diff mode" × {local, branch}
- "💬 bubble click opens edit popover in diff mode" × {local, branch}
- "tray pencil edit preserves the current diff mode" — locks in
  that the force-to-browse is gone for good
Police pass on PR #879 — five structural improvements after hickey + lowy
review flagged duplication and overlapping signals.

1. **`usePierreLineAnchor` extracted to `@kolu/solid-pierre`** — the rAF
   loop, shadow-DOM traversal, viewer-rect bail, and child-rect math
   were duplicated across `InlineCommentPopover` and `LineCommentMarker`.
   Now a single primitive owns the Pierre-DOM topology volatility; a
   future schema flip (e.g. `data-selected-line` rename, virtualizer
   change) lands in one file.

2. **`deepQuerySelector` moves to `@kolu/solid-pierre/lineAnchor`** —
   was housed in `LineCommentMarker.tsx`, imported by `CodeTab` and
   `InlineCommentPopover`. The volatile concept (Pierre's open shadow
   root) now lives next to the wrappers that own Pierre's lifecycle.

3. **`useCommentInteraction` hook owns all comment state** — moves 8
   handlers, 3 effects, and the per-worktree `useComments` wiring out
   of CodeTab.tsx (lost ~190 lines from CodeTab, gained ~210 in the
   focused hook). CodeTab is now layout + chrome; comment intent flows
   in via the returned API.

4. **`ComposerIntent` discriminated union collapses `editTarget` +
   `pendingEditSeed`** — previously two parallel signals had to be
   kept in sync at every edit/jump call site. One union now models
   {new, edit, jump}; the composer target and Pierre-selection seed
   are both derived via `createMemo`. Invalid states (e.g. `edit`
   target without a matching seed) are now structurally unrepresentable.

5. **`extraMenuItems` replaces `onAddComment`** — `CodeMenuFrame` /
   `useLineSelection` no longer know about comments. The frame accepts
   a generic `(range) => CodeContextMenuItem[]` callback; CodeTab
   supplies the "Add comment on path:Lrange" entry. Future
   line-anchored modes (bookmark, search-pin) plug in without modifying
   the frame.

Bonus: `fileBubbles` memo (F1 from police pass) stabilizes the `<For>`
identity so a no-op `comments()`/`selectedPath()` tick doesn't churn
existing markers.

All 13 comments e2e scenarios + 39 code-tab scenarios pass (657 steps).
User reported clicking the tray pencil sometimes needed two clicks to
open the popover.

Root cause: `args.repoRoot` is `() => props.meta?.git?.repoRoot ?? null`.
`props.meta` is `store.activeMeta()`, a `createMemo` that returns a
fresh `TerminalMetadata` object on every server `onMetadataChange`
stream tick (git status, branch tip, etc.). The repoRoot **string** is
stable but the **memo** ticks. Solid's `createEffect` tracks the signal,
not the resolved value — so:

  createEffect(() => {
    void args.repoRoot();
    setIntent(null);
    setCurrentRange(null);
  });

fired on every metadata tick and wiped `intent` mid-popover-open. The
user's second click was just re-setting `intent` after the spurious
clear, which is why the popover finally appeared.

Fix: wrap each orphan-trigger in a `createMemo` (value-equality dedup)
and subscribe via `on(memo, …, { defer: true })`. Now the effects only
fire on real transitions of the user-visible value, not on every
upstream stream push. `defer: true` skips the initial-mount run, which
was already a no-op (signals are null at construction).

Same pattern applied to `commentModeOn` and `isCodeTab` effects —
boolean signals are less likely to spuriously tick today, but the
consistency keeps any future composition (e.g. wrapping `activeTab` in
a derived memo) from re-introducing the bug.

E2e: 13/13 comment scenarios still pass.
srid added a commit that referenced this pull request May 14, 2026
**The Code tab's right-click menu gains an `Open <path>:<line>` entry.**
Reviewing a diff and wanting the full file at the same line was
previously two steps — copy `path:N`, switch to browse mode,
paste-navigate. Now it's one click; the menu item dispatches through the
same pipeline a terminal-link click does.

This is **Phase 0 of #881's redo** — small user-visible win on top of a
seam Phase 1 (line-anchored comments) will reuse. The structural review
behind the redo is in #881's body and replaces #879.

### Before / after

```
Diff view: alpha
           beta            ← right-click line 2
           gamma

Before:    [Copy path]    [Copy notes.txt:2]
After:     [Copy path]    [Copy notes.txt:2]    [Open notes.txt:2]
                                                 └──┐
                                          one click ▼
                                          browse mode, file open, line 2 highlighted
```

### The seam

The terminal-link click pipeline used to require a **paired write** at
every call site:

```ts
// Terminal.tsx (before)
rightPanel.openCodeBrowser();   // ① uncollapse + tab + browse-mode
requestCodeOpen({ ref, repoRoot, cwd, targetMode: "browse" });  // ② pending
// Comment: "must fire in the same DOM-event tick or resetKey clears
//  what pendingCodeOpen is about to set."
```

The ordering hazard sat at the call site as documented discipline, which
meant every new producer (right-click _Open_, future _Comment on_)
inherited the same wall-of-text rationale. Phase 0 collapses both writes
into one function:

```ts
// Terminal.tsx (after)
openInCodeTab({ ref, repoRoot, cwd, targetMode: "browse" });
```

`openInCodeTab` (replacing `codeNavigation.ts`) encapsulates both
writes; `useRightPanel.openCodeBrowser()` is generalized to
`openCodeAt(mode: CodeTabView)` so a Phase 1 comment authored while
reviewing a diff can re-open in that diff mode rather than always
browse.

### What landed

| Layer | Change |
| --- | --- |
| Producer-facing | `openInCodeTab(req)` — single call replaces the
paired-write pattern at every site |
| Panel state | `useRightPanel.openCodeAt(mode)` — atomic three-field
preferences patch parameterized on the target sub-mode; skips the patch
when already at target |
| Menu data | `CodeContextMenuItem` becomes a discriminated union
(`kind: "copy"` vs `kind: "action"`); `handleItem` dispatches via
`ts-pattern.exhaustive()` |
| Selection hook | `useLineSelection` gains `onOpen?: Accessor<((ref) =>
void) \| undefined>`; when present and a range is selected, an _Open
path:N_ action item is emitted |
| Code tab wiring | The diff-view `CodeMenuFrame` passes `onOpen` that
routes through `openInCodeTab`. The browse-view path deliberately omits
it — the file is already on screen at line precision |

_The `Accessor<…>` shape on `onOpen` mirrors `initialRange` — buildItems
runs at menu-open time, so a host whose prop arrives late in the SolidJS
lifecycle still flips the item in._

### Test coverage

- `code-tab.feature` — new scenario: right-click in diff view, click
_Open notes.txt:2_, assert mode flipped to browse + file selected + line
2 highlighted
- `code-tab.feature` — updated multi-file diff scenario to assert the
three-item menu (`Copy path | Copy file-b.txt:1 | Open file-b.txt:1`)
- `file-ref-link.feature` — terminal-link clicks now flow through
`openInCodeTab`; all three pre-existing scenarios still pass

43/43 scenarios, 503/503 steps locally.

### Try it locally

```sh
nix run github:juspay/kolu/open-in-code-tab
```

_Generated by [`/do`](https://github.com/srid/agency) on Claude Code
(model `claude-opus-4-7`)._
@srid srid closed this May 17, 2026
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.

Comment mode: annotate any file in the Code tab, flush to clipboard

1 participant