fix: Adjust metadata ID resolution order in Deepnote data handling

[tkislan]

Updated the order of ID resolution in the DeepnoteDataConverter, DeepnoteFileChangeWatcher, and Pocket modules to prioritize '__deepnoteBlockId' over 'id'. Added a test to ensure updates are not applied when cells lack block IDs and no fallback is available.

So the problem is that VS Code's live cells have lost their original metadata.id. Let me trace exactly how getBlockIdFromMetadata resolves block IDs from live cells. The IDs come from cell.metadata, which VS Code manages internally. VS Code is known to overwrite metadata.id with its own internal cell ID.

Summary by CodeRabbit

• Bug Fixes

  • Improved consistency of notebook cell identification by changing the fallback identifier priority, reducing mismatches, stale references, and preventing unnecessary edits during snapshot processing.

• Tests

  • Added tests to validate behavior when cells lack identifiers and no fallback is available, ensuring snapshot reads occur without applying edits.

[coderabbitai]

📝 Walkthrough

Walkthrough

Reorders block ID resolution to prefer meta.__deepnoteBlockId over meta.id across four files: src/notebooks/deepnote/deepnoteDataConverter.ts, src/notebooks/deepnote/deepnoteFileChangeWatcher.ts (two occurrences), and src/platform/deepnote/pocket.ts. Two unit tests were added to src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts to assert behavior when cells lack block IDs and no fallback exists. Changes are minimal and limited to the fallback/id-precedence logic and test coverage.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Title accurately summarizes the main change: adjusting metadata ID resolution order across Deepnote data handling files.
Updates Docs	✅ Passed	This PR is an internal bug fix addressing metadata ID resolution priority when reconstructing fallback blocks—a correctness improvement that doesn't introduce user-facing functionality requiring documentation updates.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0%. Comparing base (9281088) to head (eea3550).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@     Coverage Diff     @@
##   main   #338   +/-   ##
===========================
===========================

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 1

🤖 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/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts`:
- Around line 1071-1074: Replace the fixed sleep after snapshotOnDidChange.fire
by waiting deterministically for the condition that readSnapshotCallCount
increments: instead of await new Promise(resolve => setTimeout(...)), create a
Promise that resolves when readSnapshotCallCount >= 1 (either by subscribing to
the same observable/event that triggers the read, attaching a one-time listener,
or polling readSnapshotCallCount with a short interval and overall timeout) and
await that; target the test logic around snapshotOnDidChange.fire,
debounceWaitMs and the readSnapshotCallCount assertion so the test no longer
relies on a fixed delay.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9281088 and eea3550.

📒 Files selected for processing (4)

• src/notebooks/deepnote/deepnoteDataConverter.ts
• src/notebooks/deepnote/deepnoteFileChangeWatcher.ts
• src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts
• src/platform/deepnote/pocket.ts

[coderabbitai]

Actionable comments posted: 1

🤖 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/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts`:
- Around line 1055-1081: The test needs a short grace period after the waitFor
to mirror existing patterns and avoid flakiness: after the existing waitFor(()
=> readSnapshotCallCount >= 1) call, add a delay using the shared
autoSaveGraceMs value (or await a helper that waits that duration) before
asserting snapshotApplyEditCount is 0; this should be applied in the test using
the same symbols used in the file (waitFor, autoSaveGraceMs,
snapshotOnDidChange, readSnapshotCallCount, snapshotApplyEditCount) so the
assertion that applyEdit was not called runs only after the grace period.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between eea3550 and 4577bc9.

📒 Files selected for processing (1)

• src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts