fix: create daily notes from date links - #901
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
Walkthrough
ChangesDaily note materialization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Editor as useEditorAutocomplete
participant Core as materializeDailyNote
participant Disk as createNoteIfAbsent
Editor->>Core: Materialize selected date
Core->>Disk: Create daily note if absent
Disk-->>Core: Return ensured note
Core-->>Editor: Complete selection
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src-tauri/src/db/tests.rs (1)
382-403: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract table count querying into a helper closure.
The logic for querying
notesandlinkstable counts is duplicated before and after the migration. You can optionally extract this into a small closure to make the test leaner and avoid repetition.♻️ Proposed refactor
- let counts_before: Vec<i64> = ["notes", "links"] - .iter() - .map(|table| { - conn.query_row(&format!("SELECT count(*) FROM {table}"), [], |row| { - row.get(0) - }) - .unwrap() - }) - .collect(); - - migrate(&mut conn).expect("migrate to v19"); - - let counts_after: Vec<i64> = ["notes", "links"] - .iter() - .map(|table| { - conn.query_row(&format!("SELECT count(*) FROM {table}"), [], |row| { - row.get(0) - }) - .unwrap() - }) - .collect(); - assert_eq!(counts_after, counts_before); + let get_counts = |conn: &Connection| -> Vec<i64> { + ["notes", "links"] + .iter() + .map(|table| { + conn.query_row(&format!("SELECT count(*) FROM {table}"), [], |row| { + row.get(0) + }) + .unwrap() + }) + .collect() + }; + + let counts_before = get_counts(&conn); + migrate(&mut conn).expect("migrate to v19"); + assert_eq!(get_counts(&conn), counts_before);🤖 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 `@apps/desktop/src-tauri/src/db/tests.rs` around lines 382 - 403, Extract the duplicated table-count query from the test around migrate into a local helper closure that accepts the database connection and returns counts for “notes” and “links”. Use the closure for both counts_before and counts_after while preserving the existing query and assertion behavior.
🤖 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 `@apps/desktop/src-tauri/src/db/tests.rs`:
- Around line 382-403: Extract the duplicated table-count query from the test
around migrate into a local helper closure that accepts the database connection
and returns counts for “notes” and “links”. Use the closure for both
counts_before and counts_after while preserving the existing query and assertion
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 713a3014-6b10-4df4-af5f-32042beb309b
📒 Files selected for processing (3)
apps/desktop/src-tauri/src/db/tests.rscrates/index-schema/migrations/0019_lazy_daily_backlinks.sqlcrates/index-schema/src/lib.rs
|
Can you help us understand a bit more on the context here? We should be auto-creating daily notes automatically when you type a date in or a relative time like "three days from now". |
jasonlong
changed the title
|
Ugh, for some reason my LLM is commenting on my behalf here. In a nutshell, the original issue I was seeing was this:
Leaving LLM's comment below... You’re right — I traced this back through the V1 backlink-menu contract. Selecting an ISO or generated relative-date suggestion is supposed to materialize the daily note immediately. The current V2 autocomplete inserted the normalized ISO target, but unlike the ordinary Create row, the pathless date suggestion had no I initially addressed the downstream backlink symptom in the schema, which was the wrong layer and would have made manually typed dangling date links look like backlinks to nonexistent notes. I’ve replaced that approach in dc5827d: selecting a new date suggestion now atomically creates the empty |
There was a problem hiding this comment.
Pull request overview
This PR restores the V1-style side effect for date-based wiki-link autocomplete by materializing missing daily notes when the user selects an ISO or relative-date suggestion, using the atomic no-clobber creation command and surfacing failures to the user.
Changes:
- Add
materializeDailyNote(date, generation)in@reflect/coreto atomically create an empty daily note atdaily/YYYY-MM-DD.md. - Wire the editor autocomplete selection flow to materialize “pathless” date suggestions in the background and report failures via operations UI.
- Add unit/browser tests covering exact dates, generated relative dates, existing dailies, invalid dates, and error surfacing.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/graph/create-note.ts | Adds materializeDailyNote helper that creates an empty daily note via createNoteIfAbsent. |
| packages/core/src/graph/create-note.test.ts | Adds tests for daily note materialization behavior (create, no-clobber, invalid dates). |
| packages/core/src/exports/platform.ts | Exports materializeDailyNote for desktop consumption via @reflect/core. |
| apps/desktop/src/editor/use-editor-autocomplete.ts | Triggers daily-note materialization on selecting a pathless date suggestion; surfaces failures. |
| apps/desktop/src/editor/use-editor-autocomplete.test.tsx | Adds tests for the new autocomplete selection behavior (success, no-op for existing, error surfacing). |
Comments suppressed due to low confidence (1)
apps/desktop/src/editor/use-editor-autocomplete.test.tsx:125
- This block also calls
act(...)/waitFor(...)without importing them or obtainingactfromrenderHook. Align with the existing pattern in this file by awaitingrenderHookto access itsacthelper and usingvi.waitForfor the async expectation.
const { result } = renderHook(() => useEditorAutocomplete())
const items = await result.current.onWikilinkSearch('2026-07-27')
act(() => {
items[0]!.onSelect?.()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { result } = renderHook(() => useEditorAutocomplete()) | ||
| const items = await result.current.onWikilinkSearch(query) | ||
|
|
||
| act(() => { | ||
| items[0]!.onSelect?.() | ||
| }) | ||
|
|
||
| await waitFor(() => | ||
| expect(materializeDailyNote).toHaveBeenCalledWith('2026-07-27', 7), | ||
| ) |
Summary
Context
The backlink symptom came from date autocomplete inserting the ISO link without running the V1-compatible daily creation side effect. This fixes that creation path directly rather than synthesizing backlinks for notes that do not exist.
Verification
pnpm checkpnpm buildpnpm --filter @reflect/core test src/graph/create-note.test.tspnpm --filter @reflect/desktop test src/editor/use-editor-autocomplete.test.tsxSummary by CodeRabbit