Skip to content

fix(chat-sidebar): refresh @-mention picker when workspace files change#251

Merged
mbektas merged 2 commits into
plmbr:mainfrom
pjdoland:fix/workspace-picker-refresh-on-change
May 16, 2026
Merged

fix(chat-sidebar): refresh @-mention picker when workspace files change#251
mbektas merged 2 commits into
plmbr:mainfrom
pjdoland:fix/workspace-picker-refresh-on-change

Conversation

@pjdoland
Copy link
Copy Markdown
Collaborator

Summary

The chat-sidebar `@`-mention workspace file picker scanned once on first open and cached the result for the lifetime of the chat sidebar — `workspaceFilesLoaded` was set true after the initial scan and never reset, with no `ContentsManager.fileChanged` subscription and no manual refresh affordance. Users creating a notebook in the file browser, running `touch foo.py` in a terminal, or letting Claude generate files would not see them in the picker until a full lab reload.

Solution

Subscribe to `ContentsManager.fileChanged`. When the picker is open the new file shows up after a 300ms debounce; when it's closed the cache is marked stale so the next open re-scans. A manual refresh icon button (`VscRefresh`) is added to the picker header as the escape hatch for the limitation below.

Three load-bearing decisions, all flagged by reviewers and addressed before push:

  • 300ms debounce. Coalesces the signal storm that fires when a folder is dropped or an agent creates many files. Tighter (100ms) misfires during real bursts on slower hardware; looser (1s) feels laggy on single creates.
  • In-flight gate, not force-cancel. A first draft force-canceled the previous scan on every signal. Worst-case (open picker, agent creating files at 2/s, scans taking ~1s) that produced a never-completing scan storm. The new `runWorkspaceFileScan` sets a `pendingRescanRef` flag while a scan is in flight and the running scan drains the flag at completion. Bounds the worst case to "at most one scan running + one queued."
  • Stable subscription deps. First draft used `[props]`. `ReactWidget` rebuilds `props` on every Lumino update, so the effect was reconnecting the signal on every parent render and clearing the in-flight debounce timer mid-coalesce — defeating the point. The contents manager is a singleton; captured once via `appRef`, deps now `[]`.

Plus two small a11y fixes the reviewer flagged in the same surface:

  • The picker's close button was a bare `
    ` — added `role="button"`, `tabIndex=0`, `aria-label`, and an Enter/Space keyboard handler so it matches the new refresh button.
  • Added `:focus-visible` on `.mode-tools-popover-button` so keyboard users can see focus on either button.

Testing

  • 513 pytest / tsc clean / lint clean / 104 jest, all green.
  • No new automated tests. The testable surface is the one-line `change.type !== 'save'` predicate; the React-component behavior (signal subscription, debounce, gate, in-flight drain) lacks component-test infrastructure in the codebase. Six-agent review confirmed this was the right call given the existing test patterns.
  • Manual verification: created files via the JupyterLab file browser, ran `touch` in a JupyterLab terminal, and had a Claude agent generate a notebook. Picker auto-refreshes in cases 1 and 2; case 3 refreshes on manual button (see Risks).

Risks / follow-ups

  • Server-side disk writes don't fire `fileChanged` (flagged by the JupyterLab-extension reviewer). Some NBI agent tools in `notebook_intelligence/built_in_toolsets.py` write to disk directly via `open()` rather than the Contents API. Those changes don't propagate through `fileChanged` to any client. The manual refresh button is the mitigation today; routing those writes through the Contents API would close the gap and is a worthwhile follow-up.
  • Incremental list update instead of full rescan — flagged by the performance reviewer as a refactor opportunity. With the 300ms debounce + in-flight gate the rescan cost is now bounded, so this is deferred.
  • Real `` vs `
    ` — flagged by the a11y reviewer. All four popover buttons in this header use the same `
    ` pattern. Migrating the whole family to real `
    ` is its own PR; this one fixes the consistency by bringing the close button up to the same a11y bar as the new refresh button.

@pjdoland pjdoland added the bug Something isn't working label May 13, 2026
Copy link
Copy Markdown
Collaborator

@mbektas mbektas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

Before this fix, the workspace file picker scanned once on first open
and cached the result for the lifetime of the chat-sidebar component.
`workspaceFilesLoaded` was set true after the initial scan and never
reset, with no `ContentsManager.fileChanged` subscription and no manual
refresh affordance. A user who created a notebook in the file browser,
ran `touch foo.py` in a terminal, or had Claude generate files would
not see them in the @-mention picker until a full lab reload.

The fix subscribes to `ContentsManager.fileChanged` and either rescans
(if the picker is open) or invalidates the cache (if it's closed).

Three design decisions:

1. **300ms debounce.** Bulk file ops (folder drag-drop, agent-driven
   notebook creation) fire many signals back-to-back. Without
   coalescing, one drop of N items would schedule N rescans. 300ms is
   the knee: drag-drops settle inside it, single user-initiated
   creates feel responsive.

2. **In-flight gate, not force-cancel.** Earlier drafts force-canceled
   the previous scan on every signal. Worst-case (open picker, agent
   creating files at 2/s, scans taking ~1s) that produced a never-
   completing scan storm. `runWorkspaceFileScan` instead sets
   `pendingRescanRef` while a scan is in flight and the running scan
   drains the flag at completion, bounding the storm to "at most one
   running + one queued."

3. **Stable subscription deps.** The first effect attempt used
   `[props]`. ReactWidget rebuilds `props` on every Lumino update, so
   the effect connected/disconnected the signal on every parent render
   and cleared the in-flight debounce timer mid-coalesce — defeating
   the whole point. The contents manager is a singleton, captured once
   via `appRef`, and the effect now uses `[]` deps.

A manual refresh button is added to the picker header (`VscRefresh`)
as the escape hatch for the gap below.

Known limitation: `ContentsManager.fileChanged` fires only on
operations that round-trip through the Contents API. Some NBI agent
tools (`built_in_toolsets.py`) write to disk directly via `open()`
and bypass the API; those changes don't auto-refresh. The manual
refresh button is the mitigation. Tracked as a follow-up to route
agent writes through the Contents API.

Drive-by a11y fix on the picker's close button: it was a bare
`<div onClick>`. Now mirrors the refresh button's affordances — role,
tabIndex, aria-label, Enter/Space keyboard handler. A
`:focus-visible` style is added to `.mode-tools-popover-button` so
keyboard users can see focus on either button.

No new tests. The testable surface is a one-line predicate
(`change.type !== 'save'`); the React-component-level behavior
(signal subscription, debounce, gate) lacks component-test
infrastructure in the codebase. Manual verification: created files
via file browser, terminal, and an agent tool — picker auto-refreshes
in the first two cases and refreshes on manual button in the third.
@pjdoland pjdoland force-pushed the fix/workspace-picker-refresh-on-change branch from cb083e3 to 5682150 Compare May 16, 2026 16:03
…ball

The TestArchiveSizeCap cases were patching
notebook_intelligence.skill_github_import._get_github_token, which
doesn't exist (the module imports resolve_github_token from
notebook_intelligence.util). mock.patch raised AttributeError at
context-manager enter, breaking the suite on every fresh clone.

Patch the imported reference in the skill_github_import namespace
(resolve_github_token) so the mock takes effect inside _fetch_tarball's
call site.
@mbektas mbektas merged commit bd184e9 into plmbr:main May 16, 2026
4 checks passed
@pjdoland pjdoland deleted the fix/workspace-picker-refresh-on-change branch May 16, 2026 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants