feature | Request Saving format, integrate all tabs and static drafts - #40
feature | Request Saving format, integrate all tabs and static drafts#40MatiasFarji wants to merge 10 commits into
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 saved-item note snapshots with backend resolution, frontend append/save actions, and editor mentions for rendering, replaying, and exporting request/response content. ChangesSaved Item Note Flow
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/backend/src/schemas/savedItem.ts`:
- Line 17: The request/response save schemas are too permissive because they
reuse savedItemSourceKindSchema and still allow "draft" at runtime. Update
saveRequestSchema and saveResponseSchema to use a narrower z.enum(["history",
"replay"]) for sourceKind so only sent-item sources are accepted, while leaving
the broader savedItemSourceKindSchema for the general saved item model.
In `@packages/backend/src/utils/savedItemsDb.ts`:
- Around line 225-246: The saved-item read/delete paths in getSavedItem and
deleteSavedItemRow are only filtering by id, which can cross project boundaries.
Update these helpers to accept the current project ID from the caller and
include project_id in the SQL WHERE clause for both the SELECT and DELETE
statements. Then propagate the project ID from the API layer into these
functions so rowToSavedItem only runs on rows scoped to the active project.
- Around line 37-38: The saved-items row mapping in savedItemsDb should preserve
an unknown TLS state instead of treating SQLite NULL as false. Update the draft
parsing logic in the row-to-object conversion so draftIsTls only becomes
true/false when draft_is_tls is explicitly 1 or 0, and remains undefined when
the column is NULL or missing; then ensure the draft validation path rejects
incomplete connection data rather than defaulting to plaintext HTTP.
In `@packages/frontend/src/actions/actions.ts`:
- Around line 37-51: The save flow in actions.ts is using stale note data after
loadNote(), because notesStore.currentNote still comes from the tree snapshot
instead of the freshly fetched note. Update the load/create flow around
loadNote() and notesStore.currentNote so updatedContent is built from the
hydrated note returned by the fetch (or make loadNote() return and set the
fetched note), then pass that fresh content into addBlockToContent() before
updateNoteContent().
- Around line 299-303: The saveResponse call in actions.ts is hardcoding the
source kind as history, which causes Replay saves to lose their actual origin
metadata. Update the logic around saveResponse in the relevant action to derive
the current source kind from the active surface/route or response pane context
before calling sdk.backend.saveResponse, using the existing action flow and any
route/surface helpers in actions.ts and the Response pane registration in
index.ts as the guide.
- Around line 21-35: The open-note guard in addSavedItemToNote is too late
because callers can already reach the sdk.backend.save*() path before
currentNotePath is checked. Move the notesStore.currentNotePath precondition
ahead of any backend save in addSavedItemToNote and the related action handlers
(including the multi-select/request flows) so no saved-item row is created when
no note is open. Use the existing addSavedItemToNote and sdk.backend.save*()
call sites to relocate the check consistently before persistence.
In
`@packages/frontend/src/components/content/editor/extensions/mentions/mention-saved-item.ts`:
- Around line 325-363: The onSessionCreate subscription in mention-saved-item’s
replay flow is not cleaned up on createSession failure, leaving a live handler
that can fire on later unrelated sessions. Update the try/catch around the
sdk.replay.onSessionCreate subscription so the handler is always stopped on
every failure path, including the catch block and any early returns, while
keeping the existing handler.stop calls before openTab/goTo and before the “no
longer available” toast.
In
`@packages/frontend/src/components/content/editor/extensions/mentions/mention-session-trigger.ts`:
- Around line 79-80: The draft persistence path in mention-session-trigger
should not decode GraphQL Blob data with atob() directly, since that can corrupt
UTF-8 text before it reaches saveDraftRequest(). Update the code in the
mention-session-trigger flow to reuse the shared decodeRawBlob() helper from
httpEncoding.ts when reading sessionResponse.replaySession.activeEntry.raw, so
the saved draft content is decoded consistently and safely.
In `@packages/frontend/src/composables/useNoteModal.ts`:
- Around line 74-99: The note modal save helper in trySaveCurrentReplayRequest
still returns early when the active replay entry has no requestId, so replay
drafts never flow into the new saved-item path. Update this logic to mirror
mention-session-trigger.ts by handling draft sessions with
sdk.backend.saveDraftRequest(...) when entry.requestId is missing, then continue
returning the savedItemId/sessionLabel from the saved result. Keep the existing
currentSession, replaySessionEntries, and saveRequest branches intact for
non-draft replays.
In `@packages/frontend/src/utils/currentRoute.ts`:
- Around line 21-27: `trackCurrentRoute` only updates `currentRoute` on future
`onPageChange` events, so it misses the page that is already open when the SDK
starts. Seed `currentRoute` inside `trackCurrentRoute(sdk)` before registering
the listener by reading the current route from `sdk.navigation` (for example via
a synchronous getter like `getCurrentRoute` if available), then keep the
existing `onPageChange` handler to update it on subsequent navigations.
In `@packages/shared/src/index.ts`:
- Around line 67-90: The saved-item contract in the shared types still treats
replay saves as refId-only, which prevents `getSavedItem` from reconstructing a
replay request after the original tab request is deleted. Update the model
around `SavedItem`/`getSavedItem` so replay-saved requests persist the same
snapshot data as drafts (raw request content plus host/port/TLS connection
info), or generalize the existing draft snapshot fields to cover replay items as
well, and ensure the save/load flow continues to prefer `refId` when the live
request exists but falls back to the persisted snapshot when it does not.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fd256432-e9db-4bbe-9611-394a7929dbb1
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
packages/backend/package.jsonpackages/backend/src/api/index.tspackages/backend/src/api/savedItem.tspackages/backend/src/index.tspackages/backend/src/schemas/savedItem.tspackages/backend/src/utils/savedItemsDb.tspackages/frontend/package.jsonpackages/frontend/src/actions/actions.tspackages/frontend/src/components/content/editor/NoteEditor.vuepackages/frontend/src/components/content/editor/extensions/mentions/mention-request.tspackages/frontend/src/components/content/editor/extensions/mentions/mention-saved-item.tspackages/frontend/src/components/content/editor/extensions/mentions/mention-session-trigger.tspackages/frontend/src/components/shared/SearchNoteViewer.vuepackages/frontend/src/composables/useNoteModal.tspackages/frontend/src/index.tspackages/frontend/src/utils/caido.tspackages/frontend/src/utils/currentRoute.tspackages/frontend/src/utils/httpEncoding.tspackages/frontend/src/utils/nativeEditor.tspackages/frontend/src/utils/noteUtils.tspackages/shared/src/index.ts
💤 Files with no reviewable changes (2)
- packages/frontend/src/utils/caido.ts
- packages/frontend/src/components/content/editor/extensions/mentions/mention-request.ts
|
@MatiasFarji Coderabbit put some good review already. I would say anything that breaks backward compatibility without a migration is a non starter to be merged. @amrelsagaei from our team can help you build a migration system. I know @bebiksior you might have opinion on this since you rewrote the plugin a while back. |
…tead of SQLite, add optional upgrade of requests
…tead of SQLite, add optional upgrade of requests, incomplete commit
|
Hi @Sytten ! Thank you for your feedback, i'm a little bit newbie at OpenSource as i told @bebiksior in Redocs, so sorry about that :'), about the retrocompatibility, totally right, i actually made it breaking functionality because the new version of Caido kinda broke the original func, and i though the requests got corrupted (Therefore unrecoverable) so i didn't put much effort in that, but after checking out the bug was just in the viewer of the request, the actual requests was still linked. So i've made a new commit making retrocompatibility and also solving that problem with the patch, as you can check in the screenshots. I've also restored all saving data to .json format as it was initially, because i noticed that change unnecessary, probably half of the errors of CodeRabbit are solved with that, i'm going to check each one after the commit and work on them. @amrelsagaei and @bebiksior Please feel free to test them and tell me what you think. I think that, the added funcs are just an easy improvement, but specially on the replay tab, i think that saving static states of the requests is better than saving the link to the draft, because it may change a lot and from my own experience, when i save a request i usually want to save that static request. 
I'll keep you informed about the fixes. |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/frontend/src/components/content/editor/extensions/mentions/mention-saved-item.ts (1)
201-249: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClear replay state when a refresh marks the item unavailable.
After a successful load,
replayRequestId,draftConnection, andcurrentRawTextremain populated. If a laterrefreshEditorsreload returnsError,!found, or throws, the UI says “Unavailable” but double-click can still replay the stale previous target.Suggested fix
+ const clearReplayTarget = () => { + replayRequestId = undefined; + draftConnection = undefined; + currentRawText = undefined; + savedReplaySessionId = undefined; + savedSessionLabel = undefined; + }; + const loadSavedItem = async (savedItem: SavedItem): Promise<void> => { + clearReplayTarget(); try { const result = await sdk.backend.getSavedItem(savedItem);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/frontend/src/components/content/editor/extensions/mentions/mention-saved-item.ts` around lines 201 - 249, The loadSavedItem flow in mention-saved-item.ts leaves stale replay state behind when a refresh makes the item unavailable, so clear the saved replay fields whenever sdk.backend.getSavedItem returns Error, !found, or throws. Update loadSavedItem to reset replayRequestId, draftConnection, savedReplaySessionId, savedSessionLabel, and currentRawText before returning on those paths, and make sure the unavailable UI state cannot still replay the previous item.
♻️ Duplicate comments (1)
packages/frontend/src/composables/useNoteModal.ts (1)
87-96: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle unsent replay drafts here too.
Line 88 still drops active replay entries without
requestId, so the note modal saves only text and shows “No active replay session found” for drafts. Mirror the draftSavedItemconstruction used by the legacy upgrade path.Proposed fix
- const entry = sdk.replay.getEntry(activeEntryId); - if (!entry.requestId) return undefined; + const activeEntry = sessionResponse?.replaySession?.activeEntry; + const entry = sdk.replay.getEntry(activeEntryId); + if (!entry.requestId) { + const connection = activeEntry?.connection; + if (typeof connection?.host !== "string") return undefined; + + return { + kind: "request", + refId: "", + sourceKind: "draft", + draftRaw: atob(activeEntry?.raw ?? ""), + draftHost: connection.host, + draftPort: connection.port, + draftIsTls: connection.isTLS, + replaySessionId: currentSession.id, + sessionLabel: currentSession.name, + }; + } return {Also applies to: 112-120
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/frontend/src/composables/useNoteModal.ts` around lines 87 - 96, The note modal flow in useNoteModal should not return undefined for replay entries that lack requestId; instead, mirror the draft SavedItem construction used in the legacy upgrade path so unsent replay drafts are still treated as an active replay source. Update the active-entry handling around sdk.replay.getEntry and the replay draft mapping logic used by the existing upgrade path helpers so the modal can save notes for draft entries and avoid the “No active replay session found” state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/backend/src/api/note.ts`:
- Line 5: The import in note.ts is not formatted according to the configured
Prettier import layout and will fail lint. Update the shared import that brings
in Folder, Note, NoteContent, NoteContentItem, Result, and TreeNode so it
matches the project’s Prettier formatting conventions, using the existing import
statement as the location to adjust.
In `@packages/backend/src/api/savedItem.ts`:
- Around line 112-119: The saved item response is forcing a missing parent
request ID into an empty string, which can look like a real request ID
downstream. Update the response construction in the savedItem handler to omit
requestId entirely when item.parentRequestId is absent, and only include it when
a value exists so ResolvedSavedItem stays consistent with its optional requestId
field.
- Around line 50-62: The draft request branch in savedItem handling is dropping
sessionLabel, unlike the non-draft request path, so preserve that field in the
object returned from the saved draft request logic. Update the response built in
the draft branch of the saved item API so it includes item.sessionLabel
alongside the existing replaySessionId and label fields, keeping the behavior
consistent with the request branch.
In `@packages/backend/src/schemas/note.ts`:
- Around line 19-22: The appendToNoteSchema definition currently allows any
arbitrary block object, so malformed payloads can slip through; tighten the
block field in the note schema to validate against the shared note-content
contract used by NoteContentItem, ensuring at least the required type field (and
any other expected shape) is enforced. Update the block validator in
appendToNoteSchema so it matches the actual note content structure rather than
using an unconstrained passthrough object.
In
`@packages/frontend/src/components/content/editor/extensions/mentions/mention-request.ts`:
- Around line 156-158: The toast message in mention-request.ts is triggering a
Prettier/ESLint formatting issue because of its string style. Update the
showToast call in the legacy request upgrade path to match the project’s
preferred formatting for strings and escaping, keeping the message content the
same. Use the sdk.window.showToast call site as the reference point and ensure
the resulting line passes lint without changing behavior.
- Around line 297-310: The `refreshEditors` listener in `mention-request` is
being removed too broadly, which can unregister other embedded editor callbacks.
In the node view setup where `emitter.on("refreshEditors", ...)` is registered,
store the handler in a named reference and use that same reference in the
`destroy` cleanup. Update the `destroy` function so
`emitter.off("refreshEditors", handler)` removes only this node’s listener
without affecting other editors.
In
`@packages/frontend/src/components/content/editor/extensions/mentions/mention-saved-item.ts`:
- Around line 254-256: The refresh listener in mention-saved-item is registered
with an anonymous callback, so it cannot be removed safely for just this node
view. Update the refreshEditors subscription to keep a stable handler reference
in the node view instance and use that same reference when calling emitter.off
during cleanup/destroy, so only this saved-item view unsubscribes. Apply the
same fix anywhere this pattern appears in the MentionSavedItem node view,
including the duplicate refreshEditors registration referenced by the other
occurrence.
- Line 304: Apply the Prettier formatting fixes in mention-saved-item,
specifically around the sessionCreateSub type declaration and the missing final
newline. Reformat the affected declarations in the relevant block so they match
the project's Prettier style, and ensure the file ends with a newline; use the
sessionCreateSub and sdk.replay.onSessionCreate references to locate the exact
spots.
---
Outside diff comments:
In
`@packages/frontend/src/components/content/editor/extensions/mentions/mention-saved-item.ts`:
- Around line 201-249: The loadSavedItem flow in mention-saved-item.ts leaves
stale replay state behind when a refresh makes the item unavailable, so clear
the saved replay fields whenever sdk.backend.getSavedItem returns Error, !found,
or throws. Update loadSavedItem to reset replayRequestId, draftConnection,
savedReplaySessionId, savedSessionLabel, and currentRawText before returning on
those paths, and make sure the unavailable UI state cannot still replay the
previous item.
---
Duplicate comments:
In `@packages/frontend/src/composables/useNoteModal.ts`:
- Around line 87-96: The note modal flow in useNoteModal should not return
undefined for replay entries that lack requestId; instead, mirror the draft
SavedItem construction used in the legacy upgrade path so unsent replay drafts
are still treated as an active replay source. Update the active-entry handling
around sdk.replay.getEntry and the replay draft mapping logic used by the
existing upgrade path helpers so the modal can save notes for draft entries and
avoid the “No active replay session found” state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3181d40e-ddf1-48b9-be1a-9a54879cb03b
📒 Files selected for processing (17)
packages/backend/src/api/index.tspackages/backend/src/api/note.tspackages/backend/src/api/savedItem.tspackages/backend/src/index.tspackages/backend/src/schemas/note.tspackages/backend/src/schemas/savedItem.tspackages/frontend/src/actions/actions.tspackages/frontend/src/components/content/editor/NoteEditor.vuepackages/frontend/src/components/content/editor/extensions/mentions/mention-request.tspackages/frontend/src/components/content/editor/extensions/mentions/mention-saved-item.tspackages/frontend/src/components/content/editor/extensions/mentions/mention-session-trigger.tspackages/frontend/src/components/shared/SearchNoteViewer.vuepackages/frontend/src/composables/useNoteModal.tspackages/frontend/src/repositories/notes.tspackages/frontend/src/stores/notes.tspackages/frontend/src/utils/noteUtils.tspackages/shared/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/frontend/src/actions/actions.ts
- packages/shared/src/index.ts
- packages/frontend/src/components/content/editor/extensions/mentions/mention-session-trigger.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/frontend/src/components/content/editor/extensions/mentions/mention-saved-item.ts (1)
201-249: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClear replay state when a refresh marks the item unavailable.
After a successful load,
replayRequestId,draftConnection, andcurrentRawTextremain populated. If a laterrefreshEditorsreload returnsError,!found, or throws, the UI says “Unavailable” but double-click can still replay the stale previous target.Suggested fix
+ const clearReplayTarget = () => { + replayRequestId = undefined; + draftConnection = undefined; + currentRawText = undefined; + savedReplaySessionId = undefined; + savedSessionLabel = undefined; + }; + const loadSavedItem = async (savedItem: SavedItem): Promise<void> => { + clearReplayTarget(); try { const result = await sdk.backend.getSavedItem(savedItem);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/frontend/src/components/content/editor/extensions/mentions/mention-saved-item.ts` around lines 201 - 249, The loadSavedItem flow in mention-saved-item.ts leaves stale replay state behind when a refresh makes the item unavailable, so clear the saved replay fields whenever sdk.backend.getSavedItem returns Error, !found, or throws. Update loadSavedItem to reset replayRequestId, draftConnection, savedReplaySessionId, savedSessionLabel, and currentRawText before returning on those paths, and make sure the unavailable UI state cannot still replay the previous item.
♻️ Duplicate comments (1)
packages/frontend/src/composables/useNoteModal.ts (1)
87-96: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle unsent replay drafts here too.
Line 88 still drops active replay entries without
requestId, so the note modal saves only text and shows “No active replay session found” for drafts. Mirror the draftSavedItemconstruction used by the legacy upgrade path.Proposed fix
- const entry = sdk.replay.getEntry(activeEntryId); - if (!entry.requestId) return undefined; + const activeEntry = sessionResponse?.replaySession?.activeEntry; + const entry = sdk.replay.getEntry(activeEntryId); + if (!entry.requestId) { + const connection = activeEntry?.connection; + if (typeof connection?.host !== "string") return undefined; + + return { + kind: "request", + refId: "", + sourceKind: "draft", + draftRaw: atob(activeEntry?.raw ?? ""), + draftHost: connection.host, + draftPort: connection.port, + draftIsTls: connection.isTLS, + replaySessionId: currentSession.id, + sessionLabel: currentSession.name, + }; + } return {Also applies to: 112-120
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/frontend/src/composables/useNoteModal.ts` around lines 87 - 96, The note modal flow in useNoteModal should not return undefined for replay entries that lack requestId; instead, mirror the draft SavedItem construction used in the legacy upgrade path so unsent replay drafts are still treated as an active replay source. Update the active-entry handling around sdk.replay.getEntry and the replay draft mapping logic used by the existing upgrade path helpers so the modal can save notes for draft entries and avoid the “No active replay session found” state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/backend/src/api/note.ts`:
- Line 5: The import in note.ts is not formatted according to the configured
Prettier import layout and will fail lint. Update the shared import that brings
in Folder, Note, NoteContent, NoteContentItem, Result, and TreeNode so it
matches the project’s Prettier formatting conventions, using the existing import
statement as the location to adjust.
In `@packages/backend/src/api/savedItem.ts`:
- Around line 112-119: The saved item response is forcing a missing parent
request ID into an empty string, which can look like a real request ID
downstream. Update the response construction in the savedItem handler to omit
requestId entirely when item.parentRequestId is absent, and only include it when
a value exists so ResolvedSavedItem stays consistent with its optional requestId
field.
- Around line 50-62: The draft request branch in savedItem handling is dropping
sessionLabel, unlike the non-draft request path, so preserve that field in the
object returned from the saved draft request logic. Update the response built in
the draft branch of the saved item API so it includes item.sessionLabel
alongside the existing replaySessionId and label fields, keeping the behavior
consistent with the request branch.
In `@packages/backend/src/schemas/note.ts`:
- Around line 19-22: The appendToNoteSchema definition currently allows any
arbitrary block object, so malformed payloads can slip through; tighten the
block field in the note schema to validate against the shared note-content
contract used by NoteContentItem, ensuring at least the required type field (and
any other expected shape) is enforced. Update the block validator in
appendToNoteSchema so it matches the actual note content structure rather than
using an unconstrained passthrough object.
In
`@packages/frontend/src/components/content/editor/extensions/mentions/mention-request.ts`:
- Around line 156-158: The toast message in mention-request.ts is triggering a
Prettier/ESLint formatting issue because of its string style. Update the
showToast call in the legacy request upgrade path to match the project’s
preferred formatting for strings and escaping, keeping the message content the
same. Use the sdk.window.showToast call site as the reference point and ensure
the resulting line passes lint without changing behavior.
- Around line 297-310: The `refreshEditors` listener in `mention-request` is
being removed too broadly, which can unregister other embedded editor callbacks.
In the node view setup where `emitter.on("refreshEditors", ...)` is registered,
store the handler in a named reference and use that same reference in the
`destroy` cleanup. Update the `destroy` function so
`emitter.off("refreshEditors", handler)` removes only this node’s listener
without affecting other editors.
In
`@packages/frontend/src/components/content/editor/extensions/mentions/mention-saved-item.ts`:
- Around line 254-256: The refresh listener in mention-saved-item is registered
with an anonymous callback, so it cannot be removed safely for just this node
view. Update the refreshEditors subscription to keep a stable handler reference
in the node view instance and use that same reference when calling emitter.off
during cleanup/destroy, so only this saved-item view unsubscribes. Apply the
same fix anywhere this pattern appears in the MentionSavedItem node view,
including the duplicate refreshEditors registration referenced by the other
occurrence.
- Line 304: Apply the Prettier formatting fixes in mention-saved-item,
specifically around the sessionCreateSub type declaration and the missing final
newline. Reformat the affected declarations in the relevant block so they match
the project's Prettier style, and ensure the file ends with a newline; use the
sessionCreateSub and sdk.replay.onSessionCreate references to locate the exact
spots.
---
Outside diff comments:
In
`@packages/frontend/src/components/content/editor/extensions/mentions/mention-saved-item.ts`:
- Around line 201-249: The loadSavedItem flow in mention-saved-item.ts leaves
stale replay state behind when a refresh makes the item unavailable, so clear
the saved replay fields whenever sdk.backend.getSavedItem returns Error, !found,
or throws. Update loadSavedItem to reset replayRequestId, draftConnection,
savedReplaySessionId, savedSessionLabel, and currentRawText before returning on
those paths, and make sure the unavailable UI state cannot still replay the
previous item.
---
Duplicate comments:
In `@packages/frontend/src/composables/useNoteModal.ts`:
- Around line 87-96: The note modal flow in useNoteModal should not return
undefined for replay entries that lack requestId; instead, mirror the draft
SavedItem construction used in the legacy upgrade path so unsent replay drafts
are still treated as an active replay source. Update the active-entry handling
around sdk.replay.getEntry and the replay draft mapping logic used by the
existing upgrade path helpers so the modal can save notes for draft entries and
avoid the “No active replay session found” state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3181d40e-ddf1-48b9-be1a-9a54879cb03b
📒 Files selected for processing (17)
packages/backend/src/api/index.tspackages/backend/src/api/note.tspackages/backend/src/api/savedItem.tspackages/backend/src/index.tspackages/backend/src/schemas/note.tspackages/backend/src/schemas/savedItem.tspackages/frontend/src/actions/actions.tspackages/frontend/src/components/content/editor/NoteEditor.vuepackages/frontend/src/components/content/editor/extensions/mentions/mention-request.tspackages/frontend/src/components/content/editor/extensions/mentions/mention-saved-item.tspackages/frontend/src/components/content/editor/extensions/mentions/mention-session-trigger.tspackages/frontend/src/components/shared/SearchNoteViewer.vuepackages/frontend/src/composables/useNoteModal.tspackages/frontend/src/repositories/notes.tspackages/frontend/src/stores/notes.tspackages/frontend/src/utils/noteUtils.tspackages/shared/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/frontend/src/actions/actions.ts
- packages/shared/src/index.ts
- packages/frontend/src/components/content/editor/extensions/mentions/mention-session-trigger.ts
🛑 Comments failed to post (8)
packages/backend/src/api/note.ts (1)
5-5: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Apply the Prettier import layout.
Line 5 currently violates the configured Prettier rule and will fail lint.
Proposed fix
-import type { Folder, Note, NoteContent, NoteContentItem, Result, TreeNode } from "shared"; +import type { + Folder, + Note, + NoteContent, + NoteContentItem, + Result, + TreeNode, +} from "shared";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import type { Folder, Note, NoteContent, NoteContentItem, Result, TreeNode, } from "shared";🧰 Tools
🪛 ESLint
[error] 5-5: Replace
·Folder,·Note,·NoteContent,·NoteContentItem,·Result,·TreeNode·with⏎··Folder,⏎··Note,⏎··NoteContent,⏎··NoteContentItem,⏎··Result,⏎··TreeNode,⏎(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/api/note.ts` at line 5, The import in note.ts is not formatted according to the configured Prettier import layout and will fail lint. Update the shared import that brings in Folder, Note, NoteContent, NoteContentItem, Result, and TreeNode so it matches the project’s Prettier formatting conventions, using the existing import statement as the location to adjust.Source: Linters/SAST tools
packages/backend/src/api/savedItem.ts (2)
50-62: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve
sessionLabelfor saved draft requests.The draft branch drops
item.sessionLabel, while the non-draft request branch returns it. This loses the named-tab metadata needed for consistent replay navigation.Proposed fix
replaySessionId: item.replaySessionId, + sessionLabel: item.sessionLabel, label: item.label,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.return ok({ found: true, kind: "request", sourceKind: "draft", raw: item.draftRaw, draftConnection: { host: item.draftHost, port: item.draftPort, isTls: item.draftIsTls, }, replaySessionId: item.replaySessionId, sessionLabel: item.sessionLabel, label: item.label, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/api/savedItem.ts` around lines 50 - 62, The draft request branch in savedItem handling is dropping sessionLabel, unlike the non-draft request path, so preserve that field in the object returned from the saved draft request logic. Update the response built in the draft branch of the saved item API so it includes item.sessionLabel alongside the existing replaySessionId and label fields, keeping the behavior consistent with the request branch.
112-119: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Omit missing parent request IDs instead of returning
"".Line 117 serializes a missing
parentRequestIdas an empty request ID. SincerequestIdis optional inResolvedSavedItem, omit it to avoid downstream replay attempts using an invalid ID.Proposed fix
return ok({ found: true, kind: "response", sourceKind: item.sourceKind, raw: response.data.response.raw, - requestId: item.parentRequestId ?? "", + ...(item.parentRequestId ? { requestId: item.parentRequestId } : {}), label: item.label, });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.return ok({ found: true, kind: "response", sourceKind: item.sourceKind, raw: response.data.response.raw, ...(item.parentRequestId ? { requestId: item.parentRequestId } : {}), label: item.label, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/api/savedItem.ts` around lines 112 - 119, The saved item response is forcing a missing parent request ID into an empty string, which can look like a real request ID downstream. Update the response construction in the savedItem handler to omit requestId entirely when item.parentRequestId is absent, and only include it when a value exists so ResolvedSavedItem stays consistent with its optional requestId field.packages/backend/src/schemas/note.ts (1)
19-22: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Verify append callers send TipTap-like blocks with a required `type`. rg -n -C3 '\bappendToNote\s*\(' packagesRepository: caido-community/NotesPlusPlus
Length of output: 1849
🏁 Script executed:
#!/bin/bash set -euo pipefail # Map the relevant files first, then inspect the smallest useful slices. git ls-files | rg '(^|/)(note\.ts|notes\.ts|shared.*|types.*)$' printf '\n--- packages/backend/src/schemas/note.ts ---\n' cat -n packages/backend/src/schemas/note.ts printf '\n--- packages/backend/src/api/note.ts (around appendToNote) ---\n' sed -n '200,280p' packages/backend/src/api/note.ts printf '\n--- packages/frontend/src/repositories/notes.ts ---\n' cat -n packages/frontend/src/repositories/notes.ts printf '\n--- packages/frontend/src/stores/notes.ts (appendBlockToNote) ---\n' sed -n '500,560p' packages/frontend/src/stores/notes.ts printf '\n--- shared NoteContentItem definition search ---\n' rg -n -C3 'export (type|interface) NoteContentItem|type NoteContentItem|interface NoteContentItem' packagesRepository: caido-community/NotesPlusPlus
Length of output: 13327
Validate
blockagainst the note-content contract.z.object({}).passthrough()accepts{}and arbitrary objects, so malformed RPC payloads can still be appended to note JSON even though the sharedNoteContentItemshape requires at leasttype.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/schemas/note.ts` around lines 19 - 22, The appendToNoteSchema definition currently allows any arbitrary block object, so malformed payloads can slip through; tighten the block field in the note schema to validate against the shared note-content contract used by NoteContentItem, ensuring at least the required type field (and any other expected shape) is enforced. Update the block validator in appendToNoteSchema so it matches the actual note content structure rather than using an unconstrained passthrough object.packages/frontend/src/components/content/editor/extensions/mentions/mention-request.ts (2)
156-158: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the Prettier-reported string style.
ESLint reports this line as a formatting error, so the lint step can fail.
Proposed fix
- "This note has a legacy request reference. Click \"Upgrade\" to switch it to the newer format.", + 'This note has a legacy request reference. Click "Upgrade" to switch it to the newer format.',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.sdk.window.showToast( 'This note has a legacy request reference. Click "Upgrade" to switch it to the newer format.', { variant: "warning" },🧰 Tools
🪛 ESLint
[error] 157-157: Replace
"This·note·has·a·legacy·request·reference.·Click·\"Upgrade\"·to·switch·it·to·the·newer·format."with'This·note·has·a·legacy·request·reference.·Click·"Upgrade"·to·switch·it·to·the·newer·format.'(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/frontend/src/components/content/editor/extensions/mentions/mention-request.ts` around lines 156 - 158, The toast message in mention-request.ts is triggering a Prettier/ESLint formatting issue because of its string style. Update the showToast call in the legacy request upgrade path to match the project’s preferred formatting for strings and escaping, keeping the message content the same. Use the sdk.window.showToast call site as the reference point and ensure the resulting line passes lint without changing behavior.Source: Linters/SAST tools
297-310: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Verify eventBus off() semantics and all refreshEditors registrations. fd -a 'eventBus' packages/frontend/src --exec sed -n '1,160p' {} rg -n -C 3 'emitter\.(on|off)\("refreshEditors"' packages/frontend/srcRepository: caido-community/NotesPlusPlus
Length of output: 3859
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the event bus implementation/type surface and all refreshEditors registrations. sed -n '1,120p' packages/frontend/src/lib/eventBus.ts 2>/dev/null || true sed -n '1,120p' packages/frontend/src/eventBus.ts 2>/dev/null || true rg -n -C 2 'refreshEditors' packages/frontend/srcRepository: caido-community/NotesPlusPlus
Length of output: 3130
Unregister the exact
refreshEditorshandler.mitt.off("refreshEditors")clears every listener for that event, so destroying one node view can drop the refresh callback for the other embedded editors. Keep a handler reference and pass it tooff.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/frontend/src/components/content/editor/extensions/mentions/mention-request.ts` around lines 297 - 310, The `refreshEditors` listener in `mention-request` is being removed too broadly, which can unregister other embedded editor callbacks. In the node view setup where `emitter.on("refreshEditors", ...)` is registered, store the handler in a named reference and use that same reference in the `destroy` cleanup. Update the `destroy` function so `emitter.off("refreshEditors", handler)` removes only this node’s listener without affecting other editors.packages/frontend/src/components/content/editor/extensions/mentions/mention-saved-item.ts (2)
254-256: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unsubscribe only this node view’s refresh handler.
The anonymous
refreshEditorscallback cannot be removed precisely. Store the handler and pass it tooff; otherwise destroying one saved-item node can remove other node views’ refresh listeners or fail to clean up this one, depending on the emitter implementation.Suggested fix
- emitter.on("refreshEditors", () => { + const refreshEditorsHandler = () => { loadSavedItem(item); - }); + }; + emitter.on("refreshEditors", refreshEditorsHandler); @@ dom: container, destroy: () => { - emitter.off("refreshEditors"); + emitter.off("refreshEditors", refreshEditorsHandler); },Also applies to: 349-351
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/frontend/src/components/content/editor/extensions/mentions/mention-saved-item.ts` around lines 254 - 256, The refresh listener in mention-saved-item is registered with an anonymous callback, so it cannot be removed safely for just this node view. Update the refreshEditors subscription to keep a stable handler reference in the node view instance and use that same reference when calling emitter.off during cleanup/destroy, so only this saved-item view unsubscribes. Apply the same fix anywhere this pattern appears in the MentionSavedItem node view, including the duplicate refreshEditors registration referenced by the other occurrence.
304-304: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Apply the Prettier fixes before merge.
ESLint reports formatting failures on the
sessionCreateSubtype declaration and missing final newline.Also applies to: 356-356
🧰 Tools
🪛 ESLint
[error] 304-304: Replace
·ReturnType<typeof·sdk.replay.onSessionCreate>with⏎············|·ReturnType<typeof·sdk.replay.onSessionCreate>⏎···········(prettier/prettier)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/frontend/src/components/content/editor/extensions/mentions/mention-saved-item.ts` at line 304, Apply the Prettier formatting fixes in mention-saved-item, specifically around the sessionCreateSub type declaration and the missing final newline. Reformat the affected declarations in the relevant block so they match the project's Prettier style, and ensure the file ends with a newline; use the sessionCreateSub and sdk.replay.onSessionCreate references to locate the exact spots.Source: Linters/SAST tools
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
packages/frontend/src/composables/useNoteModal.ts (1)
86-95: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMirror the replay snapshot flow here instead of falling back to
requestId.Lines 86-95 still reject draft entries and only persist
requestIdfor non-draft replay entries, so note-modal attachments are neither draft-safe nor the static replay snapshot described by this PR. That means draft tabs still hit the misleading “No active replay session found” path, and persisted replay attachments can depend on mutable tab state again. Please build the saved item from the active entry’s raw payload/connection metadata here, or reuse the same replay-save helper used by the main “Save Request to Note” action.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/frontend/src/composables/useNoteModal.ts` around lines 86 - 95, The note-modal attachment builder in useNoteModal should not gate on entry.requestId or fall back to mutable replay state; it needs to mirror the replay snapshot save flow. Update the logic around getEntry(activeEntryId) to build the saved item from the active entry’s raw payload/connection metadata, or reuse the same replay-save helper used by the main Save Request to Note action, so draft entries are supported and the persisted replay attachment is static.packages/frontend/src/actions/actions.ts (1)
262-264: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep replay responses persisted as
history.Line 262 regresses the response contract already clarified in this PR: replay-pane saves still use the canonical history/request ID, so tagging them as
"replay"makes the saved metadata disagree with the reference being stored. KeepsourceKindhardcoded to"history"until replay responses have a distinct backend resolution path.Suggested fix
- sourceKind: window.location.hash === "`#/replay`" ? "replay" : "history", + sourceKind: "history",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/frontend/src/actions/actions.ts` around lines 262 - 264, The `sourceKind` assignment in `actions.ts` is incorrectly switching to `"replay"` based on `window.location.hash`, which makes replay-pane saves diverge from the canonical history/request ID they still use. Update the logic in the action that builds the saved response metadata so `sourceKind` remains hardcoded to `"history"` for now, and keep the existing `label: ctx.request.path` behavior unchanged. Only revisit this in the future when replay responses have a separate backend resolution path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/frontend/src/actions/actions.ts`:
- Line 270: The file ending in actions.ts is missing the required trailing
newline, which is causing Prettier to fail. Update the end of the file near the
closing export block in actions.ts so the last line is followed by a final
newline character and the file ends cleanly.
In `@packages/frontend/src/components/shared/NoteFloatModal.vue`:
- Line 164: The NoteFloatModal.vue file is missing the required final newline,
which is causing Prettier to fail. Update the file so it ends with a newline
after the closing template tag, preserving the existing content and formatting.
In `@packages/frontend/src/composables/useNoteModal.ts`:
- Line 207: The file ending in useNoteModal.ts is missing the required trailing
newline, which is causing Prettier to fail. Update the end of the file so it
terminates with a newline character after the final closing brace in
useNoteModal, keeping the rest of the content unchanged.
In `@packages/frontend/src/index.ts`:
- Line 169: The file ending in the frontend entrypoint is missing the required
trailing newline, which keeps Prettier/lint unhappy. Update the end of the
module around the closing brace so the file terminates with a final newline and
no other content changes are needed.
---
Duplicate comments:
In `@packages/frontend/src/actions/actions.ts`:
- Around line 262-264: The `sourceKind` assignment in `actions.ts` is
incorrectly switching to `"replay"` based on `window.location.hash`, which makes
replay-pane saves diverge from the canonical history/request ID they still use.
Update the logic in the action that builds the saved response metadata so
`sourceKind` remains hardcoded to `"history"` for now, and keep the existing
`label: ctx.request.path` behavior unchanged. Only revisit this in the future
when replay responses have a separate backend resolution path.
In `@packages/frontend/src/composables/useNoteModal.ts`:
- Around line 86-95: The note-modal attachment builder in useNoteModal should
not gate on entry.requestId or fall back to mutable replay state; it needs to
mirror the replay snapshot save flow. Update the logic around
getEntry(activeEntryId) to build the saved item from the active entry’s raw
payload/connection metadata, or reuse the same replay-save helper used by the
main Save Request to Note action, so draft entries are supported and the
persisted replay attachment is static.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9ffca914-075a-406b-a89f-3c284f8775eb
📒 Files selected for processing (5)
packages/frontend/src/actions/actions.tspackages/frontend/src/components/content/editor/extensions/mentions/mention-session-trigger.tspackages/frontend/src/components/shared/NoteFloatModal.vuepackages/frontend/src/composables/useNoteModal.tspackages/frontend/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/frontend/src/components/content/editor/extensions/mentions/mention-session-trigger.ts
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/frontend/src/components/content/editor/extensions/mentions/mention-request.ts (1)
168-170: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStop
dblclickpropagation from the Upgrade button too.Line 170 only stops
click; a double-click on Upgrade can still bubble to the container’sdblclickhandler and navigate to replay during migration.Proposed fix
migrateButton.addEventListener("click", async (event) => { // Avoid triggering the dblclick-to-replay handler below. event.stopPropagation(); migrateButton.disabled = true; migrateButton.textContent = "Upgrading..."; @@ } }); + + migrateButton.addEventListener("dblclick", (event) => { + event.stopPropagation(); + });Also applies to: 303-307
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/frontend/src/components/content/editor/extensions/mentions/mention-request.ts` around lines 168 - 170, The Upgrade button click handler in mention-request should also block double-click navigation, not just single-click propagation. Update the migrateButton listener (and the other matching Upgrade handler around the second occurrence) so it explicitly stops dblclick bubbling as well, using the same event flow protection already applied in the surrounding mention container handlers to prevent the replay navigation from firing during migration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/backend/src/schemas/note.ts`:
- Around line 19-29: The note node schema currently allows any nested content,
so malformed child nodes can still be written through appendToNoteSchema. Update
noteContentItemSchema to validate nested children recursively by using the
recursive note node shape (for example with z.lazy) and make sure the recursive
schema is referenced from the shared NoteContentItem definition so content is
always a tree of valid note nodes.
In
`@packages/frontend/src/components/content/editor/extensions/mentions/mention-request.ts`:
- Around line 157-160: The legacy mention render path in mention-request.ts is
showing a warning toast every time the editor renders, which causes repeated
notifications. Update the toast logic around the legacy request reference
handling so it only fires once per editor/session (or remove it and rely on the
inline Upgrade button), using the existing mention rendering flow in the
mention-request extension to gate repeated calls to sdk.window.showToast.
- Line 108: The new decodeRawBlob call in mention-request should be reformatted
to match the repository’s Prettier style. Update the draftRaw assignment in the
mention-request logic so the expression follows the standard multiline
formatting used by the codebase, keeping the existing behavior unchanged while
satisfying ESLint/Prettier.
---
Outside diff comments:
In
`@packages/frontend/src/components/content/editor/extensions/mentions/mention-request.ts`:
- Around line 168-170: The Upgrade button click handler in mention-request
should also block double-click navigation, not just single-click propagation.
Update the migrateButton listener (and the other matching Upgrade handler around
the second occurrence) so it explicitly stops dblclick bubbling as well, using
the same event flow protection already applied in the surrounding mention
container handlers to prevent the replay navigation from firing during
migration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 835a7637-bc5a-4419-a356-8494934f7f3c
📒 Files selected for processing (8)
packages/backend/src/api/note.tspackages/backend/src/api/savedItem.tspackages/backend/src/schemas/note.tspackages/frontend/src/actions/actions.tspackages/frontend/src/components/content/editor/extensions/mentions/mention-request.tspackages/frontend/src/components/content/editor/extensions/mentions/mention-saved-item.tspackages/frontend/src/composables/useNoteModal.tspackages/frontend/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/backend/src/api/note.ts
- packages/frontend/src/index.ts
- packages/frontend/src/components/content/editor/extensions/mentions/mention-saved-item.ts
- packages/backend/src/api/savedItem.ts
- packages/frontend/src/actions/actions.ts
- packages/frontend/src/composables/useNoteModal.ts
…ng, note legacy upgrading dblclick associated to caido-community#40 (review)
|
Hi @amrelsagaei ! I've finished the fixes on CodeRabbit recommendations, i've tested the flow and it seems to work all fine, there is a funct that i broke at one point (Attach context on the notes) that i wasn't aware of, but i solved it, although it still works only on the replay context (I'm not sure if its necessary to include it in the other context, i think the plugin tab on right click is enough). By the way, my first exp with CodeRabbit, it found a few nice bugs :). |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/backend/src/schemas/note.ts (1)
28-42: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider caching per-depth schemas instead of rebuilding on each descent.
Each call to
buildNoteContentItemSchema(depth)constructs a freshz.lazy(() => z.object(...))instance. Since onlyMAX_CONTENT_DEPTH + 1distinct depth levels exist, precomputing/memoizing schemas by depth (e.g., an array indexed by depth) would avoid re-allocating schema objects for every node during a deep parse, at the cost of a small amount of extra code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/schemas/note.ts` around lines 28 - 42, The recursive schema builder in buildNoteContentItemSchema currently creates a new z.lazy/z.object instance for every descent, even though only a fixed set of depth levels is needed. Add per-depth memoization or precompute the schemas in note.ts so each depth reuses the same schema instance, and keep the MAX_CONTENT_DEPTH branching behavior intact while updating buildNoteContentItemSchema to return cached schemas instead of rebuilding them.packages/frontend/src/utils/jsonToMarkdown.ts (2)
277-303: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDe-duplicate saved-item fetches by key before calling the backend.
savedItemMentions(fromcollectSavedItemMentions) is not de-duplicated, so if the same saved item is referenced by multiple mentions in the note,sdk.backend.getSavedItemis called once per mention instead of once per unique item, wasting IPC/GraphQL round-trips (and duplicatedecodeRawBlobwork) proportional to reference count.♻️ Proposed dedup before fetching
- if (savedItemMentions.length > 0) { + const uniqueSavedItems = Array.from( + new Map(savedItemMentions.map((item) => [savedItemKey(item), item])).values(), + ); + if (uniqueSavedItems.length > 0) { const p = (async () => { - const fetchPromises = savedItemMentions.map(async (item) => { + const fetchPromises = uniqueSavedItems.map(async (item) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/frontend/src/utils/jsonToMarkdown.ts` around lines 277 - 303, The saved-item fetch loop in jsonToMarkdown should deduplicate mentions before calling sdk.backend.getSavedItem, since savedItemMentions may contain repeated references. Update the fetch logic around collectSavedItemMentions handling to group by savedItemKey (or an equivalent unique identifier) and fetch each unique item only once, then populate savedItemContentMap from the deduped set to avoid repeated backend calls and duplicate decodeRawBlob work.
281-297: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNon-exception failures from
getSavedItemaren't logged.The
catchblock logs fetch exceptions, but whenresult.kind !== "Success"(a resolved failure response rather than a thrown error), nothing is logged — only the silent "unavailable" fallback appears downstream. This makes backend-reported failures indistinguishable from legitimately deleted/unavailable items during debugging.🔧 Proposed fix to add logging for non-success results
const result = await sdk.backend.getSavedItem(item); if (result.kind === "Success" && result.value.found) { const resolved = result.value; const raw = resolved.sourceKind === "draft" ? resolved.raw : decodeRawBlob(resolved.raw); savedItemContentMap.set(savedItemKey(item), { raw, kind: resolved.kind, label: item.label, }); + } else if (result.kind !== "Success") { + console.error("Failed to resolve saved item:", result); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/frontend/src/utils/jsonToMarkdown.ts` around lines 281 - 297, The saved item lookup in jsonToMarkdown’s getSavedItem flow only logs thrown exceptions, so resolved non-success responses are silently treated as missing data. Update the try block around sdk.backend.getSavedItem(item) to detect result.kind !== "Success" and emit a console.error there with the item context and returned failure details before continuing with the fallback behavior. Keep the existing catch logging for exceptions, and use the same savedItemKey/item.label flow so the failure source is easy to locate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/backend/src/schemas/note.ts`:
- Around line 28-42: The recursive schema builder in buildNoteContentItemSchema
currently creates a new z.lazy/z.object instance for every descent, even though
only a fixed set of depth levels is needed. Add per-depth memoization or
precompute the schemas in note.ts so each depth reuses the same schema instance,
and keep the MAX_CONTENT_DEPTH branching behavior intact while updating
buildNoteContentItemSchema to return cached schemas instead of rebuilding them.
In `@packages/frontend/src/utils/jsonToMarkdown.ts`:
- Around line 277-303: The saved-item fetch loop in jsonToMarkdown should
deduplicate mentions before calling sdk.backend.getSavedItem, since
savedItemMentions may contain repeated references. Update the fetch logic around
collectSavedItemMentions handling to group by savedItemKey (or an equivalent
unique identifier) and fetch each unique item only once, then populate
savedItemContentMap from the deduped set to avoid repeated backend calls and
duplicate decodeRawBlob work.
- Around line 281-297: The saved item lookup in jsonToMarkdown’s getSavedItem
flow only logs thrown exceptions, so resolved non-success responses are silently
treated as missing data. Update the try block around
sdk.backend.getSavedItem(item) to detect result.kind !== "Success" and emit a
console.error there with the item context and returned failure details before
continuing with the fallback behavior. Keep the existing catch logging for
exceptions, and use the same savedItemKey/item.label flow so the failure source
is easy to locate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 83078e28-5d22-41ab-8efc-9c35c3fb0eae
📒 Files selected for processing (3)
packages/backend/src/schemas/note.tspackages/frontend/src/components/content/editor/extensions/mentions/mention-request.tspackages/frontend/src/utils/jsonToMarkdown.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/frontend/src/components/content/editor/extensions/mentions/mention-request.ts
2 participants
BREAKING FUNCTIONALITY
I'm proposing a change in the integration of the requests to the notes, this integration would break the saved requests in the Notes ++ of previous versions.
Improvements to request saver, integrating in 2 different ways:
Summary by CodeRabbit