[codex] Support saved block entries - #8
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Free Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
| id: "bookmarked-entries", | ||
| name: "Saved Entries", |
There was a problem hiding this comment.
🚩 Settings panel ID renamed from 'bookmarked-pages' to 'bookmarked-entries'
The settings panel section ID changed from bookmarked-pages to bookmarked-entries (src/index.ts:34). This is safe if extensionAPI.settings.panel.create replaces the entire panel on load (typical behavior). However, if the Roam extension framework tracks individual settings items by ID across loads, the old bookmarked-pages entry could remain orphaned in stored settings. Worth confirming this does not cause duplicate settings panel sections for users upgrading from the previous version.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Non-blocking feedback (1)
- Add targeted tests for the new block-entry lifecycle —
tests/quickSwitcher.test.ts
The utility suite now covers parsing and block title derivation, but it still doesn’t exercise the highest-risk mixed flows introduced in this PR (addBlock+ target-based dedupe + open behavior).
Consider adding focused coverage for: rejecting page UIDs inaddBlock, ensuring block targets route throughopenBlock, and mixed page/block duplicate+shortcut scenarios after sanitization.
If you want me to push fixes, reply with item numbers (for example: please fix 1).
Summary
Validation