feat: Expose the environment <-> project mapping to agents.#326
feat: Expose the environment <-> project mapping to agents.#326
Conversation
📝 WalkthroughWalkthroughAdded DeepnoteExtensionSidecarWriter to persist per-notebook Deepnote environment mappings into a deepnote.json sidecar under an editor-settings folder. The writer serializes writes, reads existing sidecar content, resolves environment details (venvPath, projectId), and updates the sidecar on mapper events, environment changes, notebook opens, and during activation. IDeepnoteNotebookEnvironmentMapper gained getAllMappings() and two events (onDidSetEnvironment, onDidRemoveEnvironment) which the writer subscribes to. The writer is registered as an IExtensionSyncActivationService in the service registry (the binding 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 env details (venvPath, projectId)
EnvMgr-->>Writer: env details / projectId
Writer->>Writer: update mapping in-memory
Writer->>Editor: write deepnote.json
EnvMgr->>Writer: onEnvironmentChanged / onEnvironmentRemoved
Writer->>FS: reconcile or remove stale entries
🚥 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.
src/kernels/deepnote/environments/deepnoteExtensionSidecarWriter.node.ts
Show resolved
Hide resolved
src/kernels/deepnote/environments/deepnoteExtensionSidecarWriter.node.ts
Show resolved
Hide resolved
src/kernels/deepnote/environments/deepnoteExtensionSidecarWriter.node.ts
Show resolved
Hide resolved
src/kernels/deepnote/environments/deepnoteExtensionSidecarWriter.unit.test.ts
Show resolved
Hide resolved
src/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node.ts
Show resolved
Hide resolved
src/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node.ts
Outdated
Show resolved
Hide resolved
src/kernels/deepnote/environments/deepnoteNotebookEnvironmentMapper.node.ts
Show resolved
Hide resolved
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.
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