Skip to content

Refresh dependent notebook diagnostics after text changes - #1917

Open
Maxim Gagiev (maximilliangrand) wants to merge 1 commit into
microsoft:mainfrom
maximilliangrand:fix/notebook-dependent-diagnostic-refresh
Open

Maxim Gagiev (maximilliangrand) wants to merge 1 commit into
microsoft:mainfrom
maximilliangrand:fix/notebook-dependent-diagnostic-refresh

Conversation

@maximilliangrand

Copy link
Copy Markdown

Fixes #1837.

After a notebook cell edit, unchanged cells with inter-file dependencies wait in the background scheduler, which delays each pull by 500 ms. Reuse the existing notebook-wide refresh path for text changes so matching cells are pulled together after the change notification is sent. Keep the capability and on-change guards, language/visibility filtering, and per-event deduplication.

The integration regression edits one of three Python cells and checks that all three are pulled exactly once while an unrelated JavaScript cell is excluded. It holds background timers to prevent a delayed pull from hiding the regression.

Validation on macOS arm64 with Node 22.17.0 and VS Code Insiders:

  • npm run compile passed.
  • Client and client integration-test lint passed.
  • npm --prefix client-node-tests test: 225 passed.
  • On unchanged main with the new regression: 224 passed, 1 failed because only the edited cell was pulled.

Browser and other package test suites were not run. Implemented and independently reviewed with Codex assistance.

Copilot AI balanced review requested due to automatic review settings October 23, 2026 12:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The narrowly scoped change reuses the existing guarded refresh path, and the regression directly covers the reported delay with no unresolved findings.

No open issues 🎉

What changed in this PR

Addresses #1837 by refreshing dependent notebook cells together after a text change, avoiding the background scheduler’s per-cell delay.

Changes:

  • Extends the existing notebook-wide refresh path to text edits while preserving capability guards, cell filtering, and deduplication.
  • Adds a regression test that blocks background timers and verifies exactly one pull per Python cell, excluding an unrelated JavaScript cell.
File Description
client/​src/​common/​diagnostic.ts Triggers notebook-wide dependent-cell refreshes on text changes.
client-node-tests/​src/​integration.test.ts Tests prompt dependent-cell pulls, filtering, and deduplication.

Review effort: Balanced

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.

Notebook dependent-cell diagnostic refresh is serialized at 500 ms per cell

2 participants