feat: Expose the environment <-> project mapping to agents.#326
feat: Expose the environment <-> project mapping to agents.#326
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds DeepnoteExtensionSidecarWriter: a new service that persists per-notebook Deepnote environment mappings into a sidecar file named deepnote.json in an editor settings folder. It listens to IDeepnoteNotebookEnvironmentMapper events (onDidSetEnvironment, onDidRemoveEnvironment), notebook-open events, and IDeepnoteEnvironmentManager changes; resolves environment details and project IDs; serializes writes via an internal queue; reads existing sidecar and per-project metadata on activation to reconcile mappings; and writes updates to the filesystem. The writer is registered as an IExtensionSyncActivationService (the registration appears duplicated). Sequence DiagramsequenceDiagram
participant Notebook
participant Mapper
participant Writer
participant EnvMgr
participant FS as Filesystem
participant Editor as EditorSettings
Notebook->>Mapper: setEnvironmentForNotebook(uri, envId)
Mapper->>Mapper: persist mapping
Mapper->>Mapper: emit onDidSetEnvironment(uri, envId)
Mapper->>Writer: onDidSetEnvironment event
Writer->>Writer: enqueue write
Writer->>FS: read deepnote.json (if exists)
Writer->>EnvMgr: resolve environment (venvPath, projectId)
EnvMgr-->>Writer: environment details / projectId
Writer->>Writer: update in-memory reverse map
Writer->>Editor: write deepnote.json
EnvMgr->>Writer: onEnvironmentChanged / onEnvironmentRemoved
Writer->>Writer: reconcile or remove stale entries
Notebook->>Writer: onNotebookOpen(uri)
Writer->>EnvMgr: resolve notebook projectId
EnvMgr-->>Writer: projectId (or none)
Writer->>Editor: update sidecar if needed
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #326 +/- ##
===========================
===========================
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/kernels/deepnote/environments/deepnoteExtensionSidecarWriter.node.ts`:
- Around line 71-98: In handleEnvironmentsChanged, wrap the body of the for
(const [projectId, entry] of Object.entries(sidecar.mappings)) loop in a
per-iteration try/catch so a single bad entry doesn't abort the sweep: inside
enqueueWrite keep the existing logic that looks up environment via
this.environmentManager.getEnvironment(entry.environmentId) and updates or
deletes sidecar.mappings, but surround that per-project logic with try { ... }
catch (err) { logger.warn('[SidecarWriter] Failed processing mapping', {
projectId, entry, err }); continue; } so errors are logged with projectId and
entry and the loop continues; ensure the changed flag is only set when a mapping
is actually modified or deleted.
- Around line 202-244: In syncExistingMappings(), make per-entry error handling
so one failure doesn't abort the whole sync: wrap each iteration of the first
for loop (where you call this.environmentManager.getEnvironment and await
this.readProjectIdFromFile and set this.fsPathToProjectId) in a try/catch and
log the specific fsPath/projectId error, and also wrap each iteration inside the
enqueueWrite callback's for loop (where you assign
sidecar.mappings[entry.projectId]) in a try/catch and log entry-specific errors;
keep behavior otherwise the same so successful entries proceed even if some
fail.
- Around line 254-263: enqueueWrite currently chains onto this.writeQueue so if
the async write throws the queue becomes rejected and future writes never run;
change the chaining to preserve a recoverable queue while still propagating the
actual write error to the caller by doing: create a local promise op =
this.writeQueue.then(async () => { const sidecar = await this.readSidecar();
const changed = await mutate(sidecar); if (changed) await
this.writeSidecar(sidecar); }); then set this.writeQueue = op.catch(() => { /*
swallow to keep queue alive (optionally log) */ }); and return op so callers of
enqueueWrite still see the original error while the internal writeQueue is
recovered for subsequent calls.
In
`@src/kernels/deepnote/environments/deepnoteExtensionSidecarWriter.unit.test.ts`:
- Around line 212-237: The test currently asserts sidecar.mappings by checking
individual properties; replace those per-property asserts with a single
assert.deepStrictEqual comparing the entire mappings object returned by
parseSidecar() to the expected object (e.g., { 'proj-1': { environmentId:
'env-1', ... }, 'proj-2': { environmentId: 'env-2', ... } }) inside the
'multiple projects accumulate entries in a single sidecar' test — locate the
parseSidecar() usage and the existing asserts on sidecar.mappings and swap them
for one assert.deepStrictEqual(sidecar.mappings, expectedMappings) to match
guidelines.
In `@src/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node.ts`:
- Around line 85-90: The getter getAllMappings currently returns the internal
Map (mappings) directly which allows external mutation; change getAllMappings to
return a defensive copy (e.g., new Map(this.mappings) or an immutable
representation) so callers cannot modify the internal mappings state, ensuring
the returned type remains ReadonlyMap<string,string> while copying the data from
mappings before returning.
- Around line 4-7: The imports currently mix third‑party and local modules;
split them by adding a blank line between the 'vscode' imports (EventEmitter,
Uri, Memento) and the local project imports (IDisposableRegistry,
IExtensionContext and logger) so third‑party (inversify, vscode) stay grouped
together and local imports (platform/common/types, platform/logging) sit below
separated by one empty line.
- Around line 19-24: Reorder the event fields so private members come together
and public members come together, each group alphabetized: place the private
EventEmitter fields _onDidRemoveEnvironment and _onDidSetEnvironment next to
each other in alphabetical order, and then place the public event accessors
onDidRemoveEnvironment and onDidSetEnvironment together in alphabetical order;
update their declarations so accessibility groups are contiguous and sorted as
required.
In `@src/kernels/deepnote/types.ts`:
- Around line 354-368: Reorder the event declarations in the Deepnote types so
they follow the project's alphabetical ordering rule: move the
onDidRemoveEnvironment property to appear before onDidSetEnvironment (keeping
getAllMappings where it is); update the declaration order of
onDidRemoveEnvironment and onDidSetEnvironment in the interface so accessibility
remains the same but the members are alphabetized.
---
Duplicate comments:
In
`@src/kernels/deepnote/environments/deepnoteExtensionSidecarWriter.unit.test.ts`:
- Around line 374-400: Replace the individual property assertions in the test
"activation syncs multiple projects including closed notebooks" with a single
deep equality assertion: build the expected mappings object (keys 'proj-1' and
'proj-2' mapping to objects containing environmentId 'env-1' and 'env-2') and
call assert.deepStrictEqual(sidecar.mappings, expectedMappings) instead of the
separate assert.strictEqual checks; update the assertions around parseSidecar()
and Object.keys(...) accordingly to use deepStrictEqual for the full mappings
comparison.
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@src/kernels/deepnote/environments/deepnoteExtensionSidecarWriter.unit.test.ts`:
- Around line 234-236: Replace the three separate assertions with a single deep
equality check against the expected mappings object: instead of
assert.strictEqual/Object.keys checks and property checks, use
assert.deepStrictEqual on sidecar.mappings and an explicit expected object
(containing proj-1 and proj-2 entries with their environmentId values) so the
entire mappings structure is compared at once; update the test in
deepnoteExtensionSidecarWriter.unit.test.ts to use assert.deepStrictEqual for
the sidecar.mappings assertion.
- Around line 397-399: Replace the three assertions that individually check
mapping length and each mapping property (the calls using assert.strictEqual and
indexing into sidecar.mappings) with a single assert.deepStrictEqual that
compares sidecar.mappings to the expected mappings object; update the test
around the symbol sidecar.mappings and remove the individual assertions so the
test uses assert.deepStrictEqual(sidecar.mappings, { 'proj-1': { environmentId:
'env-1' }, 'proj-2': { environmentId: 'env-2' } }) to validate the entire
mappings shape in one assertion.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/kernels/deepnote/environments/deepnoteExtensionSidecarWriter.unit.test.ts`:
- Around line 279-283: Extract the inline numeric timeouts used in the unit
tests (e.g., usages of await new Promise((resolve) => setTimeout(resolve, 200))
and the 100 ms variants) into named top-level constants (for example
ASYNC_OP_DELAY_MS and SHORT_DELAY_MS) declared near the top of the test module,
then replace the inline literals with those constants in the tests (including
the occurrences at the shown snippet and the other instances noted at 349-352,
437-439, 486-488) so the delays are clear and consistent across the file.
- Around line 426-429: Replace the piecemeal assertions on sidecar.mappings with
a single deep object comparison: use assert.deepStrictEqual to compare the
parsed sidecar.mappings (from parseSidecar() → sidecar.mappings) against the
expected mappings object (e.g., an object containing only 'proj-good' present
and 'proj-missing' absent) so the test verifies the entire mappings shape in one
assertion rather than multiple property checks.
In `@src/kernels/deepnote/types.ts`:
- Around line 352-368: The interface member ordering is out of alphabetical
order; move getAllMappings so the methods/events are sorted alphabetically and
grouped consistently—reorder the interface so getAllMappings appears before
getNotebooksUsingEnvironment, and ensure onDidRemoveEnvironment and
onDidSetEnvironment remain alphabetized among events; update the sequence around
the unique symbols getAllMappings, getNotebooksUsingEnvironment,
onDidRemoveEnvironment, and onDidSetEnvironment to satisfy the "methods, fields
and properties first by accessibility then alphabetically" rule.
---
Duplicate comments:
In `@src/kernels/deepnote/environments/deepnoteExtensionSidecarWriter.node.ts`:
- Around line 240-248: The loop that writes entries inside enqueueWrite can be
aborted by a single bad entry; update the callback passed to enqueueWrite (the
async sidecar => { ... } function) to wrap each iteration in a try/catch around
processing an individual entry (referencing entries, entry.projectId,
entry.environmentId, entry.venvPath and sidecar.mappings) so a failure for one
entry is caught, logged/handled, and the loop continues for remaining entries;
preserve the overall return true from the callback so the enqueueWrite completes
even if some entries fail.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/kernels/deepnote/environments/deepnoteExtensionSidecarWriter.node.ts`:
- Around line 48-353: The private members in DeepnoteExtensionSidecarWriter are
out of the required accessibility+alphabetical order; reorder the class so
members are grouped by accessibility (public, then private) and within each
group sorted alphabetically — keep fields first (fsPathToProjectId, writeQueue),
then constructor, then public methods (activate), then all private methods
alphabetically: enqueueWrite, getSidecarUri, handleEnvironmentsChanged,
handleNotebookOpened, handleRemoveEnvironment, handleSetEnvironment,
readProjectIdFromFile, readSidecar, resolveProjectId, syncExistingMappings,
writeSidecar (or rename/reorder any handlers so their names sort correctly)
while preserving existing implementations and references.
- Around line 344-351: writeSidecar currently calls workspace.fs.writeFile
directly which fails when the sidecar directory tree (e.g.
.vscode/.cursor/.antigravity) does not exist; before writing, compute the parent
directory of the sidecar Uri (use path.dirname(uri.path) or Uri utilities) and
call workspace.fs.createDirectory(parentDirUri) to ensure the folder exists,
then proceed with workspace.fs.writeFile; update the writeSidecar method (and
keep getSidecarUri usage) to create the directory first.
---
Duplicate comments:
In `@src/kernels/deepnote/environments/deepnoteExtensionSidecarWriter.node.ts`:
- Around line 263-270: Wrap each loop iteration inside the enqueueWrite callback
with a try/catch so a single bad entry cannot abort the entire write;
specifically, inside the async callback passed to enqueueWrite that iterates
over entries and mutates sidecar.mappings[entry.projectId], enclose the body
that builds and assigns the mapping for each entry in try/catch, on error log or
record the failure (including entry.projectId and the thrown error) and continue
to the next entry so remaining entries are still written.
In
`@src/kernels/deepnote/environments/deepnoteExtensionSidecarWriter.unit.test.ts`:
- Around line 282-283: Replace magic numeric timeouts used in setTimeout calls
(e.g., 200 and 100 ms) with named constants declared near the top of the test
module (e.g., ASYNC_OPERATION_DELAY_MS = 200, SHORT_DELAY_MS = 100); update
every occurrence in deepnoteExtensionSidecarWriter.unit.test.ts (including the
setTimeout calls around the async waits at the shown location and the other
occurrences at the noted lines) to use these constants so the delays are
centralized and self-documenting.
- Around line 430-433: The test currently checks sidecar.mappings piecemeal;
replace the three assertions with a single object comparison using
assert.deepStrictEqual to compare sidecar.mappings to the expected mappings
object. Construct an expectedMappings literal that contains the exact mapping
for 'proj-good' (and omits 'proj-missing') and then call
assert.deepStrictEqual(sidecar.mappings, expectedMappings) after parseSidecar()
so the assertion is atomic and uses the parseSidecar() / sidecar.mappings
symbols.
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/kernels/deepnote/environments/deepnoteExtensionSidecarWriter.node.ts`:
- Around line 263-270: The loop inside syncExistingMappings that calls
this.enqueueWrite and iterates over entries can be aborted by one bad entry;
modify the code so each iteration over entries is wrapped in its own try/catch
(inside the async callback passed to enqueueWrite) so a failure for one entry
doesn’t stop processing others, e.g., catch and log the error (including
identifying fields like entry.projectId and entry.environmentId) and continue to
the next entry while still writing successful mappings to sidecar.mappings; keep
enqueueWrite and the shape of sidecar.mappings unchanged.
- Around line 47-356: The class DeepnoteExtensionSidecarWriter currently has
private methods and fields out of the required accessibility+alphabetical order;
reorder members so public members come first (activate, constructor), then
private fields (fsPathToProjectId, writeQueue) and then all private methods in
alphabetical order: enqueueWrite, getSidecarUri, handleEnvironmentsChanged,
handleNotebookOpened, handleRemoveEnvironment, handleSetEnvironment,
readProjectIdFromFile, readSidecar, resolveProjectId, syncExistingMappings,
writeSidecar; keep method bodies unchanged so behavior is not modified.
In
`@src/kernels/deepnote/environments/deepnoteExtensionSidecarWriter.unit.test.ts`:
- Around line 436-439: Replace the three separate assertions that check
sidecar.mappings (Object.keys length, assert.isDefined for 'proj-good',
assert.isUndefined for 'proj-missing') with a single assert.deepStrictEqual
comparing sidecar.mappings to an expectedMappings object; build expectedMappings
to contain only the 'proj-good' entry with its expected mapping value and then
call assert.deepStrictEqual(sidecar.mappings, expectedMappings) (remove the
length/isDefined/isUndefined checks). Use parseSidecar and sidecar.mappings to
locate the test.
- Around line 288-289: Several tests use inline setTimeout delays (e.g., await
new Promise((resolve) => setTimeout(resolve, 200))) — extract those magic-number
delays into top-level named constants (for example SHORT_DELAY_MS,
MEDIUM_DELAY_MS) declared near the top of the module and replace the inline
literals in deepnoteExtensionSidecarWriter.unit.test.ts; update all occurrences
mentioned (the 200ms/100ms instances and the other occurrences referenced) to
use the new constants to improve clarity and consistency.
This will create a
.vscode/deepnote.jsonfile with the project to environment mapping.{ "mappings": { "ee8ba108-9936-4b04-a0ec-8d57aceb3de8": { "environmentId": "a66fe64b-c07f-4947-9110-f3da4a05ee66", "venvPath": "/Users/artmann/Library/Application Support/Code/User/globalStorage/deepnote.vscode-deepnote/deepnote-venvs/a66fe64b-c07f-4947-9110-f3da4a05ee66" } } }Summary by CodeRabbit
New Features
Chores
Tests