fix(SUP-1717): enforce durable note journal backlinks - #41
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR refactors journal note linking to canonicalize on a "📝 Notes" section while supporting legacy "🗂️ Notes Created" sections based on note dates. It updates the audit module with optional legacy fallback, refactors link insertion with new deduplication helpers and journal file creation, integrates the changes in the Obsidian vault adapter, and adds comprehensive test coverage for both audit and linking logic. ChangesJournal Section Migration and Link Handling
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bccb863843
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const fs = require('node:fs'); | ||
| const os = require('node:os'); |
There was a problem hiding this comment.
When running npm test --prefix modules/jarvos-secondbrain, Node fails to parse this file with SyntaxError: Identifier 'fs' has already been declared because fs and os were already declared at the top of the test file. This prevents the entire journal-note-audit.test.js file from running, so the package test suite fails before it reaches these new assertions.
Useful? React with 👍 / 👎.
bccb863 to
19d6868
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@modules/jarvos-secondbrain/adapters/obsidian/src/vault-storage-adapter.js`:
- Around line 160-167: The function linkNoteToJournal is passing the
caller-supplied heading into linkNoteBacklinkToJournal (section: heading) while
returning heading: NOTES_HEADING, which allows backlinks to be written to a
non-canonical section; change the call to linkNoteBacklinkToJournal to pass
section: NOTES_HEADING (i.e., enforce NOTES_HEADING in the write path) and keep
returning heading: NOTES_HEADING so both the write and returned metadata
consistently use NOTES_HEADING; update any related local variables in
linkNoteToJournal that reference heading to use NOTES_HEADING to avoid
confusion.
In `@modules/jarvos-secondbrain/bridge/provenance/src/link-to-journal.js`:
- Around line 130-133: The filter logic for cleanedSection is keeping standalone
'-' placeholders when sectionHadLink is true; update the predicate in the
sectionLines.filter used to build cleanedSection (the function referencing
exactLinkLine and sectionHadLink) so that any line where line.trim() === '-' is
filtered out unconditionally (remove the conditional check on sectionHadLink)
while still preserving the existing exactLinkLine check, ensuring dangling
bullet placeholders are removed even if a matching link exists.
🪄 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: e740e502-df2e-4980-9d7e-9975f950324a
📒 Files selected for processing (5)
modules/jarvos-secondbrain/adapters/obsidian/src/vault-storage-adapter.jsmodules/jarvos-secondbrain/bridge/provenance/src/journal-note-audit.jsmodules/jarvos-secondbrain/bridge/provenance/src/link-to-journal.jsmodules/jarvos-secondbrain/tests/journal-note-audit.test.jsmodules/jarvos-secondbrain/tests/link-to-journal.test.js
| linkNoteToJournal({ noteTitle, date = todayDate(), heading = NOTES_HEADING }) { | ||
| if (!noteTitle) throw new Error('noteTitle is required'); | ||
| return this.appendLineToJournalSection({ | ||
| heading, | ||
| const journalPath = path.join(getVaultJournalDir(), `${date}.md`); | ||
| const result = linkNoteBacklinkToJournal({ noteTitle, section: heading, journalPath }); | ||
| return { | ||
| ...result, | ||
| heading: NOTES_HEADING, | ||
| line: `- [[${noteTitle}]]`, |
There was a problem hiding this comment.
Force canonical heading in the write path, not only in the returned metadata.
heading is still forwarded to section, so callers can write backlinks outside canonical Notes while the return object reports heading: NOTES_HEADING. This breaks canonical enforcement and creates misleading results.
Suggested patch
linkNoteToJournal({ noteTitle, date = todayDate(), heading = NOTES_HEADING }) {
if (!noteTitle) throw new Error('noteTitle is required');
const journalPath = path.join(getVaultJournalDir(), `${date}.md`);
- const result = linkNoteBacklinkToJournal({ noteTitle, section: heading, journalPath });
+ const result = linkNoteBacklinkToJournal({
+ noteTitle,
+ section: NOTES_HEADING,
+ journalPath,
+ });
return {
...result,
heading: NOTES_HEADING,
line: `- [[${noteTitle}]]`,
};🤖 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 `@modules/jarvos-secondbrain/adapters/obsidian/src/vault-storage-adapter.js`
around lines 160 - 167, The function linkNoteToJournal is passing the
caller-supplied heading into linkNoteBacklinkToJournal (section: heading) while
returning heading: NOTES_HEADING, which allows backlinks to be written to a
non-canonical section; change the call to linkNoteBacklinkToJournal to pass
section: NOTES_HEADING (i.e., enforce NOTES_HEADING in the write path) and keep
returning heading: NOTES_HEADING so both the write and returned metadata
consistently use NOTES_HEADING; update any related local variables in
linkNoteToJournal that reference heading to use NOTES_HEADING to avoid
confusion.
| const cleanedSection = sectionLines.filter((line) => { | ||
| if (exactLinkLine.test(line)) return false; | ||
| if (!sectionHadLink && line.trim() === '-') return false; | ||
| return true; |
There was a problem hiding this comment.
Remove standalone - placeholders even when the link is already present.
When a section contains both a placeholder - and an existing matching link, reruns keep the placeholder, leaving a dangling bullet in ## 📝 Notes.
Suggested patch
const cleanedSection = sectionLines.filter((line) => {
if (exactLinkLine.test(line)) return false;
- if (!sectionHadLink && line.trim() === '-') return false;
+ if (line.trim() === '-') return false;
return true;
});📝 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.
| const cleanedSection = sectionLines.filter((line) => { | |
| if (exactLinkLine.test(line)) return false; | |
| if (!sectionHadLink && line.trim() === '-') return false; | |
| return true; | |
| const cleanedSection = sectionLines.filter((line) => { | |
| if (exactLinkLine.test(line)) return false; | |
| if (line.trim() === '-') return false; | |
| return true; |
🤖 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 `@modules/jarvos-secondbrain/bridge/provenance/src/link-to-journal.js` around
lines 130 - 133, The filter logic for cleanedSection is keeping standalone '-'
placeholders when sectionHadLink is true; update the predicate in the
sectionLines.filter used to build cleanedSection (the function referencing
exactLinkLine and sectionHadLink) so that any line where line.trim() === '-' is
filtered out unconditionally (remove the conditional check on sectionHadLink)
while still preserving the existing exactLinkLine check, ensuring dangling
bullet placeholders are removed even if a matching link exists.
Summary
Verification
Paperclip: SUP-1717
Summary by CodeRabbit
New Features
Bug Fixes
Tests