Quick capture pane Apple Intelligence LLM processing (Closes #30)#50
Conversation
Adds on-device AI processing to the quick capture pane so users can dictate or type free-form text and have it parsed into structured task fields (title, body, status, dates, project, area). Uses Apple's Foundation Models framework (~3B on-device model) via a Swift FFI bridge. - Swift bridge with @generable struct for constrained structured output - Rust wrapper with system prompt builder, date validation, and case-insensitive project/area name matching - Sparkles button in title row (visible only when AI available + text entered) - Cmd+Shift+A keyboard shortcut (only active when pane is open) - Feature is completely invisible on non-Apple-Silicon/older macOS - Weak-links FoundationModels so app still launches on older systems - Stub compilation when SDK lacks FoundationModels framework Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The vault writer expects project/area names for wikilinks (e.g. "[[Acme Corp]]") but the frontend passes hash IDs from the entity index. This caused files on disk to contain wikilinks like "[[ea7d3b50f1c597cb]]" instead of "[[Acme Corp]]". Resolve hash IDs to entity titles in all four write paths: create_task, create_project, update_task, update_project. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Rewrites the task document to reflect completed work (Phases 1-3), documents learnings about Apple Intelligence ~3B model behaviour from hands-on testing and WWDC25 research, and outlines next phases: eval harness, few-shot contamination fix, deterministic date/project resolution, and polish. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Documents the end-to-end architecture, key components, prompt system, response parsing pipeline, frontend integration, logging, and known limitations. Intended as an evergreen reference for anyone working on or extending the AI enrichment feature. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds a detailed narrative walkthrough (steps 1-8) explaining how the feature works from user action through to saved task. Covers what happens at each layer and why, alongside the existing architecture diagram and component reference. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds a #[ignore] test that runs 13 inputs through the full AI pipeline (prompt building → Apple Intelligence → response parsing) and compares against expected outputs. Prints a per-case pass/fail summary. Run with: cargo test -p taskdn-desktop eval_ai -- --ignored --nocapture Uses fixed context (projects, areas, date) for reproducibility. Does not assert on failure — this is a development tool for measuring prompt quality, not a hard test. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds redundancy for non-deterministic reliability testing: multiple phrasings for "tomorrow", day-of-week scheduling, "today" immediacy, and deadline language. Also adds hallucination traps where common words (health, home, learning) should NOT match area names, and partial project name matching tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
body_empty is now Option<bool> — None means don't check. Most cases use None since body content is determined by deterministic Rust code (original text preserved when title is transformed), not by the LLM. Baseline: 11/31 passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Phase 6 now covers removing status from the LLM entirely, replacing it with keyword-based detection and two auto-ready rules (one for all quick entry, one for AI-processed entries with near-term dates). Renumbers subsequent phases. Marks Phase 5 (eval harness) as complete. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Phase 6 is now a standalone UX improvement (auto-ready when project/area + scheduled/defer set) that applies to all quick entry and can be cherry-picked independently. Phase 7 covers removing status from the LLM, keyword detection, and the AI-only near-term auto-ready rule. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds a useEffect to the quick entry pane that watches projectId, areaId, scheduled, and deferUntil. When a task has (project or area) AND (scheduled or defer-until) set, and status is still inbox, it auto-promotes to ready — the task has enough context that it doesn't need manual inbox processing. This applies to all quick entry (manual and AI-assisted) and gives immediate visual feedback via the status pill. Only promotes inbox → ready, never touches other statuses. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Removes status entirely from the Apple Intelligence @generable struct and prompt — the ~3B model was inconsistent with status and this simplifies its job (7 fields instead of 8). Status is now determined by: 1. Keyword detection in Rust (scans input for "blocked", "icebox", "in progress" etc.) — narrow, unambiguous matches only 2. Auto-ready Rule 2 in frontend (AI only): if scheduled date is within 7 days and status is inbox, promote to ready Also fixes few-shot contamination by replacing the Q1 tax return example (which was leaking "Gather all receipts first" into real responses) with a Newsletter Setup example. Adds 6 unit tests for keyword detection (deterministic, runs in normal test suite). Updates eval harness status expectations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove "already started", "working on", "stuck on", "can't proceed" as these are less explicit and could match unintended inputs. Add fat-finger variants: "waitingon", "inprogress", "ice-box". Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comparing scheduled date (midnight) against current time (e.g. 2 AM) gave a negative daysUntil, preventing promotion to ready for tasks scheduled today. Now compares date-only strings via getTodayISO() so time-of-day doesn't affect the calculation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Changes the LLM from computing YYYY-MM-DD dates to extracting raw
date expressions ("tomorrow", "next Monday", "end of March"). Rust
resolves them deterministically via the fuzzydate crate, with custom
handlers for "end of [month]", "in N weeks", ordinal suffixes, and
"on/by [day]" patterns.
Adds fuzzy project/area matching using case-insensitive substring
with a 3-char minimum to prevent spurious short matches. "Japan Trip"
now matches "Japan Trip 2025".
19 unit tests for the resolver/matcher (deterministic, normal suite).
Eval harness: 11/31 → 16/31 passing.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Task doc: marks Phase 8 complete with results (16/31 eval passing), documents what works and remaining failures, adds Phase 9 (prompt refinement) with clear next steps. Developer doc: updates architecture to reflect current state — date expression extraction, fuzzydate resolution, fuzzy project matching, deterministic status via keywords, auto-ready rules. Adds eval harness docs and unit test info. Updates known limitations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Separate area and project lists to reduce area hallucination - Add "leave area empty when project is set" guidance - Add field definitions explaining what each field represents - Add 4th few-shot example for "this afternoon" → scheduledRef: "today" - Reorder @generable: scheduledRef before dueRef (more common first) - Trim verbose instructions for smaller prompt footprint Eval improvement: ~15.5/31 → ~17.5/31 average across multiple runs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Reorder @generable: project/area now before date fields (generated while input is still fresh in context) - Add project-matching example (Garden Renovation) - Add "by next Friday" due date example - Now 5 few-shot examples (Apple guidance: <5, this is at the limit) - Trim field definitions for smaller prompt footprint Eval improvement: ~15.5/31 (50%) → ~18.3/31 (59%) average across 3 runs. Project and due date extraction notably more reliable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The process_quick_entry_text command was synchronous, which Tauri v2 runs on the main thread. The Swift FFI's DispatchSemaphore.wait() blocked the main thread for 2-3 seconds during inference, causing the beach ball cursor and preventing the spinner from rendering. Now the command is async, with the blocking FFI call wrapped in tauri::async_runtime::spawn_blocking. The main thread stays free so React can re-render (showing the spinner) and handle user input (Escape to dismiss). Extracts the synchronous logic into process_quick_entry_text_sync() which is also used by the eval harness. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Developer docs: - apple-intelligence.md: fix stale struct order, remove non-evergreen test counts, add "as of" framing for eval baseline, fix context block description - quick-panes.md: add AI shortcut, auto-ready, and Apple Intelligence sections User-facing docs: - quick-entry-pane.mdx: add auto-ready and AI processing sections - keyboard-shortcuts.mdx: add Cmd+Shift+A shortcut Task doc: mark Phase 11 complete. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- cargo fmt across all new Rust files - Fix 4 clippy warnings: needless borrow, char comparison pattern, trim before split_whitespace, dead_code allow annotation - Remove date_explore_fuzzydate debug exploration test - Prettier formatting for QuickPaneApp.tsx and apple-intelligence.md Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds macOS/aarch64 Apple Intelligence integration: Swift real/stub implementations and C header, Rust FFI wrapper and build steps, Tauri commands and AI quick-entry parsing (prompts, date resolution, fuzzy matching), frontend UI/shortcut, vault title resolution, TypeScript bindings, tests, and docs. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tdn-desktop/src/components/quick-pane/QuickPaneApp.tsx (1)
371-385:⚠️ Potential issue | 🟠 MajorDon't let the availability probe abort pane initialization.
commands.checkAppleIntelligenceAvailable()is generated asPromise<boolean>, notResult<...>, so any IPC failure rejects this wholePromise.all. That means a probe error prevents the areas/projects refresh and skips the title focus, even though AI availability is optional here.Suggested shape
- const [areasResult, projectsResult, aiResult] = await Promise.all([ - commands.listAreas(), - commands.listProjects(), - commands.checkAppleIntelligenceAvailable(), - ]) + const [areasResult, projectsResult] = await Promise.all([ + commands.listAreas(), + commands.listProjects(), + ]) + let aiResult = false + try { + aiResult = await commands.checkAppleIntelligenceAvailable() + } catch (error) { + logger.warn('Failed to check Apple Intelligence availability', { error }) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tdn-desktop/src/components/quick-pane/QuickPaneApp.tsx` around lines 371 - 385, The AI availability probe (commands.checkAppleIntelligenceAvailable()) is a plain Promise<boolean> and can reject, which currently aborts the Promise.all and prevents areas/projects loading and title focus; change the code to await areas/projects together (keep Promise.all for commands.listAreas() and commands.listProjects() returning areasResult and projectsResult) and run commands.checkAppleIntelligenceAvailable() separately in its own try/catch (or use Promise.allSettled) so failures are swallowed and do not block initialization; on success call setAiAvailable(...) and on error setAiAvailable(false) or leave unchanged so the pane still completes setup (refer to commands.listAreas, commands.listProjects, commands.checkAppleIntelligenceAvailable, areasResult, projectsResult, and setAiAvailable).
🧹 Nitpick comments (7)
tdn-desktop/src-tauri/src/vault/manager.rs (2)
709-711: Add tests for ID-to-title resolution logic.The new ID-to-title resolution in
create_task,create_project,update_task, andupdate_projectlacks test coverage. Key scenarios to test:
- Valid project/area ID resolves to title
- Invalid ID falls back to original value (graceful degradation)
- Empty string is not resolved (preserves "clear" semantics)
These tests can use
initialize_for_testto set up a vault with known projects/areas, then verify the resolution behavior.As per coding guidelines: "Write comprehensive tests for business logic".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tdn-desktop/src-tauri/src/vault/manager.rs` around lines 709 - 711, Add unit tests in the tests module that exercise the ID-to-title resolution in create_task, create_project, update_task, and update_project by using initialize_for_test to seed a vault with known projects/areas; include cases where a valid project/area ID is supplied and must resolve to the title, where an invalid/nonexistent ID is supplied and the original string is preserved, and where an empty string is provided and must remain empty (to preserve "clear" semantics). For each function (create_task, create_project, update_task, update_project) write assertions that the returned/created object contains the resolved title when appropriate and unchanged values for fallback and empty cases; reuse initialize_for_test to set up the vault and reference the specific project/area IDs and titles in your assertions. Ensure tests live in the existing #[cfg(test)] mod tests block and cleanly isolate state between cases.
511-528: Combine read lock acquisitions for consistency and efficiency.Unlike
create_task(which resolves both project and area within a single lock scope), this acquires separate read locks at lines 515 and 523. While functionally correct, combining them reduces lock overhead and maintains consistency with the create path.♻️ Suggested refactor
// Resolve project/area IDs to titles for wikilinks let mut update = update; - if let Some(ref value) = update.project { - if !value.is_empty() { - let inner = self.inner.read(); - if let Some(project) = inner.index.get_project(value) { - update.project = Some(project.title.clone()); + { + let inner = self.inner.read(); + if let Some(ref value) = update.project { + if !value.is_empty() { + if let Some(project) = inner.index.get_project(value) { + update.project = Some(project.title.clone()); + } } } - } - if let Some(ref value) = update.area { - if !value.is_empty() { - let inner = self.inner.read(); - if let Some(area) = inner.index.get_area(value) { - update.area = Some(area.title.clone()); + if let Some(ref value) = update.area { + if !value.is_empty() { + if let Some(area) = inner.index.get_area(value) { + update.area = Some(area.title.clone()); + } } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tdn-desktop/src-tauri/src/vault/manager.rs` around lines 511 - 528, The code currently takes two separate read locks on self.inner when resolving update.project and update.area; change it to acquire one read lock (let inner = self.inner.read()) before both lookups and reuse that inner for inner.index.get_project(value) and inner.index.get_area(value), removing the second self.inner.read() call so both resolutions happen under a single read lock (same approach as create_task).tdn-desktop/src/components/quick-pane/QuickPaneTitle.tsx (1)
66-66: Consider internationalizing the button title.The title attribute
"Process with AI (⇧⌘A)"is hardcoded in English. Per coding guidelines, use theuseTranslationhook for user-facing strings in React components.♻️ Suggested i18n integration
+import { useTranslation } from 'react-i18next' + export function QuickPaneTitle({ ... }: QuickPaneTitleProps) { + const { t } = useTranslation() ... <button ... - title="Process with AI (⇧⌘A)" + title={t('quickPane.processWithAI')} >Then add the translation key to your locale files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tdn-desktop/src/components/quick-pane/QuickPaneTitle.tsx` at line 66, Replace the hardcoded title string in the QuickPaneTitle component with a translated string using the useTranslation hook (import and call const { t } = useTranslation()), e.g. replace title="Process with AI (⇧⌘A)" with title={t('quickPane.processWithAI', { shortcut: '⇧⌘A' })} (or include the shortcut in the locale value), and add the key quickPane.processWithAI to your locale files for all languages.tdn-desktop/src-tauri/src/commands/mod.rs (1)
6-8: Consider module visibility.
ai_promptsandai_resolveappear to be internal helpers used only by theaimodule (per context snippets showingsuper::ai_prompts::usage). They could potentially bepub(crate) modor even private submodules ofaiif not needed externally.♻️ Optional: Reduce public surface area
pub mod ai; -pub mod ai_prompts; -pub mod ai_resolve; +pub(crate) mod ai_prompts; +pub(crate) mod ai_resolve;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tdn-desktop/src-tauri/src/commands/mod.rs` around lines 6 - 8, ai_prompts and ai_resolve are currently exported but appear to be internal helpers for the ai module; reduce their visibility by changing their declarations in commands/mod.rs from public to more restrictive (either pub(crate) mod ai_prompts / pub(crate) mod ai_resolve or plain mod ai_prompts / mod ai_resolve) or move them as private submodules inside the ai module so they become ai::ai_prompts / ai::ai_resolve; update any external references if you choose to relocate (adjust paths like super::ai_prompts::... to crate::commands::ai::... or remove `pub` uses), and ensure compilation to confirm no external callers rely on the public exports.tdn-desktop/src/components/quick-pane/QuickPaneApp.tsx (1)
259-338: Prefer a plain function over a newuseCallbackhere.This handler is newly introduced, and the local rule for this surface is to rely on React Compiler rather than manual callback memoization.
As per coding guidelines,
tdn-desktop/**/*.{tsx,ts}: Do not use manual useMemo/useCallback - rely on React Compiler for automatic memoization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tdn-desktop/src/components/quick-pane/QuickPaneApp.tsx` around lines 259 - 338, The handler handleProcessWithAI is wrapped with React.useCallback which violates the rule to avoid manual memoization; change its definition to a plain async function (e.g., const handleProcessWithAI = async () => { ... }) instead of React.useCallback(..., [...]) and remove the dependency array; keep the existing logic and calls to setIsProcessingAI, commands.processQuickEntryText, logger, and state setters intact so only the wrapper changes.tdn-desktop/src-tauri/build.rs (1)
124-130: Use captured identifiers in theseprintln!calls.The new link-search lines revert to
{}formatting. Please switch them to captured variables to match the Rust formatting rule used elsewhere in this crate.As per coding guidelines,
tdn-desktop/**/*.rs: Use modern Rust formatting with format!("{variable}") instead of older Rust string formatting styles.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tdn-desktop/src-tauri/build.rs` around lines 124 - 130, Replace the positional `{}` formatting in the three println! calls with named/captured arguments: for the first use the captured variable out_dir (referencing out_dir.display()), and for the other two use toolchain_swift_lib and sdk_swift_lib (using their .display() results) as named arguments in the corresponding println! calls in build.rs so the lines become println!("cargo:rustc-link-search=native={out_dir}", out_dir = out_dir.display()) and similarly for toolchain_swift_lib and sdk_swift_lib.tdn-desktop/src-tauri/src/commands/ai.rs (1)
204-204: Use capture formatting in the remaining new Rust messages.This file still mixes positional placeholders with capture formatting. Please convert the remaining
format!/println!calls to the repo-standard"{variable}"style for consistency.As per coding guidelines,
tdn-desktop/**/*.rs: Use modern Rust formatting with format!("{variable}") instead of older Rust string formatting styles.Also applies to: 594-640, 658-658, 1042-1075
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tdn-desktop/src-tauri/src/commands/ai.rs` at line 204, Replace the positional-formatting call format!("{}\n\n{}", original_text.trim(), body_from_ai) with capture-style formatting: first bind the trimmed value (e.g., let original_trimmed = original_text.trim();) then use format!("{original_trimmed}\n\n{body_from_ai}"); do the same for all other format!/println! usages in this file (ranges noted in the review) converting positional placeholders to the repo-standard "{variable}" capture style while preserving any method calls by assigning their results to local variables before interpolation (e.g., trim/clone results).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tdn-desktop/src-tauri/build.rs`:
- Around line 33-42: The current xcrun invocations build sdk_path by consuming
stdout without checking the process exit status; update the
Command::new("xcrun") call that populates sdk_path (and the other xcrun lookup
later in the file) to capture the Output, assert output.status.success(), and on
failure call expect or bail with a message that includes stderr (converted to
UTF-8) so failures surface the xcrun error text instead of producing empty
paths; ensure you still trim and to_string() stdout on success.
- Around line 2-3: The build script currently gates the Swift bridge via a
compile-time cfg and assumes successful xcrun outputs and uses positional
formatting; change build.rs so build_apple_intelligence_bridge() is invoked only
after checking the target via the environment variables CARGO_CFG_TARGET_OS and
CARGO_CFG_TARGET_ARCH (e.g. require CARGO_CFG_TARGET_OS=="macos" and
CARGO_CFG_TARGET_ARCH=="aarch64" at runtime) instead of #[cfg(...)]; for the
xcrun calls inside build_apple_intelligence_bridge (where Command::output() is
used), check output.status.success() and return a clear error before parsing
stdout to avoid empty/invalid paths; and update the cargo:rustc-link-search
lines that use positional formatting (e.g. the statements that format with
out_dir.display()) to the captured-identifier style
("cargo:rustc-link-search=native={out_dir}") to match the coding guidelines.
In `@tdn-desktop/src-tauri/src/commands/ai_resolve.rs`:
- Around line 155-158: The code currently models months as 30 days in the branch
that checks parts[1].starts_with("month"), which causes incorrect date
arithmetic; replace the fixed-day approach with calendar-month arithmetic (e.g.,
use chrono::Months or roll your own month/year increment logic using
Date::with_month/with_year or constructing a new NaiveDate from year/month/day
with clamping to month end) when computing today + n months for the function
containing the parts[1] month branch, ensure you handle month-end correctly
(e.g., adding 1 month to Jan 31 → Feb 28/29), and add a regression test (per
tdn-desktop/**/*.{rs} test conventions) that asserts "in 1 month" from the
anchor date yields 2026-04-24 and covers month-end drift cases.
- Around line 39-48: Preprocess the input before running custom resolvers: call
preprocess_date_expr(trimmed) first and assign to a variable (e.g., cleaned)
then pass cleaned into resolve_end_of_month and resolve_in_n_weeks (instead of
trimmed) and also use cleaned for the fuzzydate fallback, ensuring all custom
handlers see normalized input; update uses of fuzzydate and any subsequent logic
to reference the cleaned value so inputs like "by end of March" or "on end of
month" are caught by resolve_end_of_month/resolve_in_n_weeks before falling
back.
In `@tdn-desktop/src-tauri/src/commands/ai.rs`:
- Around line 275-296: The current detect_status_from_keywords function uses
substring contains on the lowered input (variable lower) which causes false
positives (e.g., "unblocked" matching "blocked"); normalize punctuation and
whitespace first (collapse multiple spaces, remove/normalize punctuation like
hyphens) and then perform whole-word or whole-phrase matching instead of raw
substrings by using token boundaries or regex word-boundary matches for each
status keyword (e.g., patterns for "blocked", "waiting on"/"waitingon",
"icebox"/"ice box"/"ice-box", "in progress"/"in-progress"/"inprogress"); update
detect_status_from_keywords to apply the normalization to lower and then test
against these whole-word/phrase patterns so only exact tokens or token sequences
trigger a status.
- Around line 111-117: The current logging emits sensitive data (trimmed,
system_prompt, response and parsed task fields); replace these info-level prints
with either redacted placeholders or guard them behind a dev-only flag (e.g.
only log when cfg!(debug_assertions) or a dedicated feature/env var like
DEV_LOGS) and/or lower them to debug level; specifically update the log
statements that reference trimmed, system_prompt, response and any parsed task
fields in this module (ai.rs) so they do not emit full user content to normal
logs, and apply the same change to the other similar blocks mentioned (around
the later response/parse handling lines).
- Around line 180-185: The parsed JSON handling uses
parsed["title"].as_str().unwrap_or(original_text) which treats "" or
whitespace-only strings as valid; change it to read the title string (e.g., let
raw_title = parsed["title"].as_str().unwrap_or(original_text)), trim it, and if
trimmed.is_empty() then fall back to original_text when constructing
ParsedQuickEntry.title; update the logic around variable title (and any use in
ParsedQuickEntry) to use this trimmed-with-fallback value so whitespace-only
titles are treated as missing.
- Around line 153-263: The parsing helpers are gated to macOS/aarch64 which
prevents normal unit tests; remove the #[cfg(all(target_os = "macos",
target_arch = "aarch64"))] attribute from strip_code_fences, parse_ai_response,
and is_essentially_same so they are compiled for all targets, then add unit
tests exercising fenced JSON parsing, blank-title handling, body de-duplication
logic, and the non-JSON fallback using these functions (targeting
parse_ai_response and is_essentially_same specifically) to ensure CI covers the
deterministic parser.
In `@tdn-desktop/src-tauri/swift/apple_intelligence.swift`:
- Around line 95-100: The FFI entrypoint processTextWithSystemPrompt (returning
UnsafeMutablePointer<AppleLLMResponse>) currently can block indefinitely and
ignores maxTokens; wrap the model/generation work in a bounded execution (e.g.,
run the FoundationModels call inside a cancellable Task or a synchronous block
with a DispatchSemaphore/RunLoop wait and a timeout) so the call returns after a
fixed duration, and wire maxTokens into the model generation parameters (or
remove the unused maxTokens parameter from the ABI) so output length is
enforced; ensure you convert a timeout/failure into a safe AppleLLMResponse
error/partial result and always free any resources before returning.
---
Outside diff comments:
In `@tdn-desktop/src/components/quick-pane/QuickPaneApp.tsx`:
- Around line 371-385: The AI availability probe
(commands.checkAppleIntelligenceAvailable()) is a plain Promise<boolean> and can
reject, which currently aborts the Promise.all and prevents areas/projects
loading and title focus; change the code to await areas/projects together (keep
Promise.all for commands.listAreas() and commands.listProjects() returning
areasResult and projectsResult) and run
commands.checkAppleIntelligenceAvailable() separately in its own try/catch (or
use Promise.allSettled) so failures are swallowed and do not block
initialization; on success call setAiAvailable(...) and on error
setAiAvailable(false) or leave unchanged so the pane still completes setup
(refer to commands.listAreas, commands.listProjects,
commands.checkAppleIntelligenceAvailable, areasResult, projectsResult, and
setAiAvailable).
---
Nitpick comments:
In `@tdn-desktop/src-tauri/build.rs`:
- Around line 124-130: Replace the positional `{}` formatting in the three
println! calls with named/captured arguments: for the first use the captured
variable out_dir (referencing out_dir.display()), and for the other two use
toolchain_swift_lib and sdk_swift_lib (using their .display() results) as named
arguments in the corresponding println! calls in build.rs so the lines become
println!("cargo:rustc-link-search=native={out_dir}", out_dir =
out_dir.display()) and similarly for toolchain_swift_lib and sdk_swift_lib.
In `@tdn-desktop/src-tauri/src/commands/ai.rs`:
- Line 204: Replace the positional-formatting call format!("{}\n\n{}",
original_text.trim(), body_from_ai) with capture-style formatting: first bind
the trimmed value (e.g., let original_trimmed = original_text.trim();) then use
format!("{original_trimmed}\n\n{body_from_ai}"); do the same for all other
format!/println! usages in this file (ranges noted in the review) converting
positional placeholders to the repo-standard "{variable}" capture style while
preserving any method calls by assigning their results to local variables before
interpolation (e.g., trim/clone results).
In `@tdn-desktop/src-tauri/src/commands/mod.rs`:
- Around line 6-8: ai_prompts and ai_resolve are currently exported but appear
to be internal helpers for the ai module; reduce their visibility by changing
their declarations in commands/mod.rs from public to more restrictive (either
pub(crate) mod ai_prompts / pub(crate) mod ai_resolve or plain mod ai_prompts /
mod ai_resolve) or move them as private submodules inside the ai module so they
become ai::ai_prompts / ai::ai_resolve; update any external references if you
choose to relocate (adjust paths like super::ai_prompts::... to
crate::commands::ai::... or remove `pub` uses), and ensure compilation to
confirm no external callers rely on the public exports.
In `@tdn-desktop/src-tauri/src/vault/manager.rs`:
- Around line 709-711: Add unit tests in the tests module that exercise the
ID-to-title resolution in create_task, create_project, update_task, and
update_project by using initialize_for_test to seed a vault with known
projects/areas; include cases where a valid project/area ID is supplied and must
resolve to the title, where an invalid/nonexistent ID is supplied and the
original string is preserved, and where an empty string is provided and must
remain empty (to preserve "clear" semantics). For each function (create_task,
create_project, update_task, update_project) write assertions that the
returned/created object contains the resolved title when appropriate and
unchanged values for fallback and empty cases; reuse initialize_for_test to set
up the vault and reference the specific project/area IDs and titles in your
assertions. Ensure tests live in the existing #[cfg(test)] mod tests block and
cleanly isolate state between cases.
- Around line 511-528: The code currently takes two separate read locks on
self.inner when resolving update.project and update.area; change it to acquire
one read lock (let inner = self.inner.read()) before both lookups and reuse that
inner for inner.index.get_project(value) and inner.index.get_area(value),
removing the second self.inner.read() call so both resolutions happen under a
single read lock (same approach as create_task).
In `@tdn-desktop/src/components/quick-pane/QuickPaneApp.tsx`:
- Around line 259-338: The handler handleProcessWithAI is wrapped with
React.useCallback which violates the rule to avoid manual memoization; change
its definition to a plain async function (e.g., const handleProcessWithAI =
async () => { ... }) instead of React.useCallback(..., [...]) and remove the
dependency array; keep the existing logic and calls to setIsProcessingAI,
commands.processQuickEntryText, logger, and state setters intact so only the
wrapper changes.
In `@tdn-desktop/src/components/quick-pane/QuickPaneTitle.tsx`:
- Line 66: Replace the hardcoded title string in the QuickPaneTitle component
with a translated string using the useTranslation hook (import and call const {
t } = useTranslation()), e.g. replace title="Process with AI (⇧⌘A)" with
title={t('quickPane.processWithAI', { shortcut: '⇧⌘A' })} (or include the
shortcut in the locale value), and add the key quickPane.processWithAI to your
locale files for all languages.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 045cb017-f7a1-4bb4-8ae5-128d16f89b33
⛔ Files ignored due to path filters (4)
docs/tasks-todo/task-x-quick-entry-ai-processing.mdis excluded by!**/*.mdtdn-desktop/docs/developer/apple-intelligence.mdis excluded by!**/*.mdtdn-desktop/docs/developer/quick-panes.mdis excluded by!**/*.mdtdn-desktop/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
tdn-desktop/src-tauri/Cargo.tomltdn-desktop/src-tauri/build.rstdn-desktop/src-tauri/src/apple_intelligence.rstdn-desktop/src-tauri/src/bindings.rstdn-desktop/src-tauri/src/commands/ai.rstdn-desktop/src-tauri/src/commands/ai_prompts.rstdn-desktop/src-tauri/src/commands/ai_resolve.rstdn-desktop/src-tauri/src/commands/mod.rstdn-desktop/src-tauri/src/lib.rstdn-desktop/src-tauri/src/vault/manager.rstdn-desktop/src-tauri/swift/apple_intelligence.swifttdn-desktop/src-tauri/swift/apple_intelligence_bridge.htdn-desktop/src-tauri/swift/apple_intelligence_stub.swifttdn-desktop/src/components/quick-pane/QuickPaneApp.tsxtdn-desktop/src/components/quick-pane/QuickPaneTitle.tsxtdn-desktop/src/components/quick-pane/useQuickPaneKeyboard.tstdn-desktop/src/lib/bindings.tswebsite/src/content/docs/desktop/quick-entry-pane.mdxwebsite/src/content/docs/reference/desktop-reference/keyboard-shortcuts.mdx
| #[cfg(all(target_os = "macos", target_arch = "aarch64"))] | ||
| build_apple_intelligence_bridge(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In a Cargo build script (build.rs), do #[cfg(target_os = "...")]and#[cfg(target_arch = "...")]evaluate for the host platform or the final compilation target? What is the recommended way to branch on the target being built insidebuild.rs?
💡 Result:
In a Cargo build script (build.rs), #[cfg(target_os = "...")] and #[cfg(target_arch = "...")] evaluate for the host platform, not the final compilation target. This is because the build script is compiled and executed on the host machine. The recommended way to branch on the target being built inside build.rs is to read the CARGO_CFG_* environment variables set by Cargo (e.g., CARGO_CFG_TARGET_OS, CARGO_CFG_TARGET_ARCH), which reflect the target platform. Alternatively, parse the TARGET environment variable, or use crates like build-target or build-rs for a typed API. Example: use std::env; fn main { let target_os = env::var("CARGO_CFG_TARGET_OS").unwrap; if target_os == "windows" { println!("cargo:rustc-link-lib=userenv"); } else if target_os == "linux" { println!("cargo:rustc-link-lib=pthread"); } }
Citations:
- 1: https://doc.rust-lang.org/cargo/reference/build-scripts.html
- 2: https://doc.rust-lang.org/nightly/cargo/reference/build-scripts.html?highlight=feature
- 3: https://doc.rust-lang.org/beta/cargo/reference/build-script-examples.html
- 4: https://doc.rust-lang.org/stable/cargo/reference/environment-variables.html
🏁 Script executed:
git ls-files "tdn-desktop/src-tauri/build.rs"Repository: dannysmith/taskdn
Length of output: 91
🏁 Script executed:
cat -n tdn-desktop/src-tauri/build.rsRepository: dannysmith/taskdn
Length of output: 6001
Gate the bridge on the Cargo target, not the build-script host.
In build.rs, #[cfg(all(target_os = "macos", target_arch = "aarch64"))] at lines 2 and 16 evaluates for the build-script host platform, not the final compilation target. An Intel macOS host building for aarch64-apple-darwin will skip the Swift bridge entirely. Use CARGO_CFG_TARGET_OS and CARGO_CFG_TARGET_ARCH environment variables at runtime instead.
Additionally, lines 33–42 and 61–70 invoke xcrun without checking exit status before consuming stdout. If either command fails, the subsequent string parsing will produce an empty or invalid path, leading to misleading errors later. Check Command::output().status.success() before using the output.
Lines 124, 127–128, and 130 use positional argument formatting ("cargo:rustc-link-search=native={}", out_dir.display()) instead of the modern captured-identifier style ("cargo:rustc-link-search=native={out_dir}") required by the coding guidelines.
Suggested fix
fn main() {
- #[cfg(all(target_os = "macos", target_arch = "aarch64"))]
- build_apple_intelligence_bridge();
+ let target_os = std::env::var("CARGO_CFG_TARGET_OS").ok();
+ let target_arch = std::env::var("CARGO_CFG_TARGET_ARCH").ok();
+
+ if matches!(
+ (target_os.as_deref(), target_arch.as_deref()),
+ (Some("macos"), Some("aarch64"))
+ ) {
+ build_apple_intelligence_bridge();
+ }
tauri_build::build()
}
-#[cfg(all(target_os = "macos", target_arch = "aarch64"))]
fn build_apple_intelligence_bridge() {
use std::path::{Path, PathBuf};
use std::process::Command;
@@ -33,10 +33,13 @@ fn build_apple_intelligence_bridge() {
let sdk_path = String::from_utf8(
Command::new("xcrun")
.args(["--sdk", "macosx", "--show-sdk-path"])
.output()
- .expect("Failed to locate macOS SDK")
+ .and_then(|output| {
+ if !output.status.success() {
+ return Err(std::io::Error::new(std::io::ErrorKind::Other, "xcrun failed"));
+ }
+ Ok(output)
+ })
+ .expect("Failed to locate macOS SDK")
.stdout,
)
@@ -61,10 +64,13 @@ fn build_apple_intelligence_bridge() {
let swiftc_path = String::from_utf8(
Command::new("xcrun")
.args(["--find", "swiftc"])
.output()
- .expect("Failed to locate swiftc")
+ .and_then(|output| {
+ if !output.status.success() {
+ return Err(std::io::Error::new(std::io::ErrorKind::Other, "xcrun failed"));
+ }
+ Ok(output)
+ })
+ .expect("Failed to locate swiftc")
.stdout,
)
@@ -124,13 +130,13 @@ fn build_apple_intelligence_bridge() {
panic!("libtool failed for Apple Intelligence bridge");
}
- println!("cargo:rustc-link-search=native={}", out_dir.display());
+ println!("cargo:rustc-link-search=native={out_dir}");
println!("cargo:rustc-link-lib=static=apple_intelligence");
println!(
- "cargo:rustc-link-search=native={}",
- toolchain_swift_lib.display()
+ "cargo:rustc-link-search=native={toolchain_swift_lib}",
);
- println!("cargo:rustc-link-search=native={}", sdk_swift_lib.display());
+ println!("cargo:rustc-link-search=native={sdk_swift_lib}");
println!("cargo:rustc-link-lib=framework=Foundation");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tdn-desktop/src-tauri/build.rs` around lines 2 - 3, The build script
currently gates the Swift bridge via a compile-time cfg and assumes successful
xcrun outputs and uses positional formatting; change build.rs so
build_apple_intelligence_bridge() is invoked only after checking the target via
the environment variables CARGO_CFG_TARGET_OS and CARGO_CFG_TARGET_ARCH (e.g.
require CARGO_CFG_TARGET_OS=="macos" and CARGO_CFG_TARGET_ARCH=="aarch64" at
runtime) instead of #[cfg(...)]; for the xcrun calls inside
build_apple_intelligence_bridge (where Command::output() is used), check
output.status.success() and return a clear error before parsing stdout to avoid
empty/invalid paths; and update the cargo:rustc-link-search lines that use
positional formatting (e.g. the statements that format with out_dir.display())
to the captured-identifier style ("cargo:rustc-link-search=native={out_dir}") to
match the coding guidelines.
| pub fn detect_status_from_keywords(input: &str) -> &'static str { | ||
| let lower = input.to_lowercase(); | ||
|
|
||
| // Check for blocked — explicit blocking language | ||
| if lower.contains("blocked") || lower.contains("waiting on") || lower.contains("waitingon") { | ||
| return "blocked"; | ||
| } | ||
|
|
||
| // Check for icebox — only very explicit mentions | ||
| if lower.contains("icebox") || lower.contains("ice box") || lower.contains("ice-box") { | ||
| return "icebox"; | ||
| } | ||
|
|
||
| // Check for in-progress — explicit mentions only | ||
| if lower.contains("in progress") | ||
| || lower.contains("in-progress") | ||
| || lower.contains("inprogress") | ||
| { | ||
| return "in-progress"; | ||
| } | ||
|
|
||
| "inbox" |
There was a problem hiding this comment.
Use whole-word/phrase matching for status detection.
These contains checks are broader than the comment claims. They will mark unblocked as blocked, and phrases like write the in progress update as in-progress. Since status is applied automatically, normalize punctuation/whitespace first and then match whole tokens or token sequences instead of raw substrings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tdn-desktop/src-tauri/src/commands/ai.rs` around lines 275 - 296, The current
detect_status_from_keywords function uses substring contains on the lowered
input (variable lower) which causes false positives (e.g., "unblocked" matching
"blocked"); normalize punctuation and whitespace first (collapse multiple
spaces, remove/normalize punctuation like hyphens) and then perform whole-word
or whole-phrase matching instead of raw substrings by using token boundaries or
regex word-boundary matches for each status keyword (e.g., patterns for
"blocked", "waiting on"/"waitingon", "icebox"/"ice box"/"ice-box", "in
progress"/"in-progress"/"inprogress"); update detect_status_from_keywords to
apply the normalization to lower and then test against these whole-word/phrase
patterns so only exact tokens or token sequences trigger a status.
build.rs: - Check xcrun exit status before consuming stdout; surface stderr on failure instead of producing empty paths ai.rs: - Word-boundary matching for "blocked" keyword to prevent false positives on "unblocked" - Empty/whitespace title fallback to original input - Remove cfg(macos/aarch64) gates from parsing helpers so they compile and are testable on all platforms - Add unit tests for strip_code_fences, is_essentially_same, parse_ai_response fallbacks (7 new tests) ai_resolve.rs: - Proper calendar month arithmetic instead of 30-day approximation (handles month-end clamping, e.g. Jan 31 + 1 month = Feb 28) - Preprocess input before custom resolvers so "by end of March" works - Add regression tests for month arithmetic and preprocessing (3 new) ai_prompts.rs / ai_resolve.rs: - Reduce module visibility from pub to pub(crate) manager.rs: - Single read lock for project+area resolution in update_task (was taking two separate locks) QuickPaneApp.tsx: - Isolate AI availability check so failures don't block areas/projects loading or title focus Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ai.rs: - Move content-bearing logs from INFO to DEBUG (only visible in dev) - Use captured-identifier format style per project guidelines - Remove useCallback wrapper (React Compiler handles memoization) apple_intelligence.swift: - Add 30-second timeout to DispatchSemaphore.wait() to prevent indefinite hangs if Foundation Models fails - Remove stale contentTagging comment vault.rs: - Add 3 unit tests for ID-to-title resolution in create_task and update_task (valid ID resolves, unknown ID preserved) QuickPaneApp.tsx: - Remove useCallback from handleProcessWithAI Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
tdn-desktop/src/components/quick-pane/QuickPaneApp.tsx (1)
276-337:⚠️ Potential issue | 🟠 MajorTreat each AI response as session-scoped, and overwrite AI-managed fields on every run.
After the async gap, this still applies results to whatever pane state is current, and it only writes back truthy values. That means a late completion can mutate a reopened pane, and empty/null parser outputs leave stale
body,due,scheduled,deferUntil,projectId, andareaIdbehind.tdn-desktop/src/lib/bindings.ts:516-524already models those fields explicitly enough to assign them directly. Invalidate the in-flight request on reset/dismiss, ignore late completions, and clearisProcessingAIwith the same session token.🛠️ Fix sketch
+ const aiSessionRef = React.useRef(0) const handleProcessWithAI = async () => { const trimmedTitle = title.trim() if (!trimmedTitle || isProcessingAI) return + const session = ++aiSessionRef.current setIsProcessingAI(true) try { const result = await commands.processQuickEntryText( trimmedTitle, projectContexts, areaPairs ) - if (result.status === 'error') { - logger.warn('AI processing failed', { error: result.error }) - setIsProcessingAI(false) - return - } + if (result.status === 'error' || session !== aiSessionRef.current) { + if (result.status === 'error') { + logger.warn('AI processing failed', { error: result.error }) + } + return + } const parsed = result.data setTitle(parsed.title) - if (parsed.body) { - setBody(parsed.body) - setShowBody(true) - } + setBody(parsed.body) + setShowBody(parsed.body.length > 0) - if (parsed.due) setDue(parsed.due) - if (parsed.scheduled) setScheduled(parsed.scheduled) - if (parsed.deferUntil) setDeferUntil(parsed.deferUntil) - if (parsed.projectId) setProjectId(parsed.projectId) - if (parsed.areaId) setAreaId(parsed.areaId) + setDue(parsed.due) + setScheduled(parsed.scheduled) + setDeferUntil(parsed.deferUntil) + setProjectId(parsed.projectId) + setAreaId(parsed.areaId) // existing status logic... } finally { - setIsProcessingAI(false) + if (session === aiSessionRef.current) { + setIsProcessingAI(false) + } } }Also invalidate the current AI session from the reset/dismiss path so reopening the pane cancels any older response before it can touch state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tdn-desktop/src/components/quick-pane/QuickPaneApp.tsx` around lines 276 - 337, Wrap the AI run in a session token to ignore late completions: before calling commands.processQuickEntryText generate/capture a local token (e.g., aiSessionToken = ++aiSessionCounter) and store it in component state; after the await, verify the token still matches the current aiSessionToken before applying any changes. When applying parsed results from processQuickEntryText, explicitly assign every AI-managed field (use setTitle, setBody, setShowBody, setStatus, setDue, setScheduled, setDeferUntil, setProjectId, setAreaId) — do not only write truthy values; set fields to null/undefined when parsed values are empty so stale values are cleared (follow the shape in bindings.ts). Also ensure reset/dismiss paths increment/clear the aiSessionCounter/token and setIsProcessingAI(false) so in-flight responses are invalidated and ignored.tdn-desktop/src-tauri/build.rs (1)
2-3:⚠️ Potential issue | 🔴 CriticalBranch on Cargo's target env, not
#[cfg], inbuild.rs.These attributes are evaluated for the build-script host. On Apple Silicon, an
x86_64-apple-darwinbuild will still compile the arm64 Swift library; on Intel macOS, anaarch64-apple-darwinbuild skips the bridge entirely. ReadCARGO_CFG_TARGET_OS/CARGO_CFG_TARGET_ARCHinmain()and remove the function-level#[cfg].Suggested fix
fn main() { - #[cfg(all(target_os = "macos", target_arch = "aarch64"))] - build_apple_intelligence_bridge(); + let target_os = std::env::var("CARGO_CFG_TARGET_OS").ok(); + let target_arch = std::env::var("CARGO_CFG_TARGET_ARCH").ok(); + + if matches!( + (target_os.as_deref(), target_arch.as_deref()), + (Some("macos"), Some("aarch64")) + ) { + build_apple_intelligence_bridge(); + } tauri_build::build() } -#[cfg(all(target_os = "macos", target_arch = "aarch64"))] fn build_apple_intelligence_bridge() {Please verify this against Cargo's build-script docs:
In Cargo build scripts (`build.rs`), do `#[cfg(target_os = "...")]` and `#[cfg(target_arch = "...")]` evaluate for the build-script host or the final compilation target? What environment variables should `build.rs` read to branch on the target being built?Also applies to: 16-17
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tdn-desktop/src-tauri/build.rs` around lines 2 - 3, The build.rs currently uses a function-level attribute #[cfg(all(target_os = "macos", target_arch = "aarch64"))] on build_apple_intelligence_bridge(), which is evaluated for the build-script host; instead remove that attribute and branch inside main() by reading Cargo's target env vars (e.g., CARGO_CFG_TARGET_OS and CARGO_CFG_TARGET_ARCH or CARGO_CFG_TARGET_ENV) from std::env to detect the target being built, then call build_apple_intelligence_bridge() when those variables indicate macos + aarch64; update any other similar guarded calls (lines calling build_apple_intelligence_bridge or similar functions) to follow the same pattern so the decision is made at runtime in build.rs rather than via #[cfg].tdn-desktop/src-tauri/swift/apple_intelligence.swift (1)
95-166:⚠️ Potential issue | 🟠 MajorTimeouts should cancel the model task, and
maxTokensis still a no-op.The 30-second wait prevents the caller from blocking forever, but the detached task keeps running after timeout, so repeated failures can pile up background model work. Also Line 99 exposes
maxTokens, but neitherrespondcall consumes it. Keep a task handle, cancel it on timeout, and either wiremaxTokensthrough or remove it from the bridge ABI.Minimal guard
- Task.detached(priority: .userInitiated) { + let task = Task.detached(priority: .userInitiated) { defer { semaphore.signal() } do { let session = LanguageModelSession( model: model, instructions: swiftSystemPrompt @@ let timeout = semaphore.wait(timeout: .now() + 30.0) if timeout == .timedOut { + task.cancel() responsePtr.pointee.error_message = duplicateCString( "Apple Intelligence timed out after 30 seconds." )Quick check:
#!/bin/bash rg -n 'maxTokens|Task\.detached|semaphore\.wait|task\.cancel' tdn-desktop/src-tauri/swift/apple_intelligence.swift🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tdn-desktop/src-tauri/swift/apple_intelligence.swift` around lines 95 - 166, The detached Task created in processTextWithSystemPrompt is never cancelled on semaphore timeout and maxTokens is unused; capture the Task handle returned by Task.detached (or use Task.init) so you can call task.cancel() if semaphore.wait times out, and ensure you propagate maxTokens into the LanguageModelSession.respond calls (or remove the maxTokens parameter from the bridge) — specifically update the Task detachment around Task.detached { ... } to assign it to a variable (e.g., let task = Task { ... }), call task.cancel() when timeout == .timedOut before returning, and wire maxTokens into session.respond (or drop the param) so the respond invocation(s) use the token limit instead of being a no-op.
🧹 Nitpick comments (1)
tdn-desktop/src-tauri/src/commands/vault.rs (1)
986-1093: Tests cover project resolution but miss area resolution paths.The new tests comprehensively verify project ID-to-title resolution for
create_taskandupdate_task. However, the manager also adds area resolution logic in:
create_task(lines 445-449 in manager.rs)create_project(lines 483-487 in manager.rs)update_task(lines 522-527 in manager.rs)update_project(lines 552-558 in manager.rs)Consider adding tests for area resolution to ensure complete coverage of the new behavior. As per coding guidelines, comprehensive tests should be written for business logic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tdn-desktop/src-tauri/src/commands/vault.rs` around lines 986 - 1093, Add tests mirroring the project-resolution tests to cover area resolution paths: implement tests for create_task, create_project, update_task, and update_project that create an Area via manager.create_project/create_project flow (or create_area helper if available), then call manager.create_task/CreateTaskOptions and manager.update_task/TaskUpdate (and the project creation/update equivalents) using area IDs; assert the resulting task.project/task.area (or project.area fields) contain the area title (not the hash ID) and that unknown area IDs are preserved verbatim. Target the manager methods create_task, create_project, update_task, and update_project when adding these assertions to ensure area ID-to-title resolution and unknown-ID preservation are covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tdn-desktop/src-tauri/swift/apple_intelligence.swift`:
- Around line 52-76: The manual JSON builder in parsedTaskToJSON(_: ParsedTask)
uses an incomplete escape table and must be replaced with a safe JSON
serializer: construct a [String: Any] (or [String: String]) dictionary from the
ParsedTask fields (title, body, project, area, scheduledRef, dueRef,
deferUntilRef) and call JSONSerialization.data(withJSONObject:options:) (or
JSONEncoder) to produce valid JSON, then convert the Data to String; remove the
custom escaping logic and ensure errors from JSONSerialization are handled or
propagated appropriately.
---
Duplicate comments:
In `@tdn-desktop/src-tauri/build.rs`:
- Around line 2-3: The build.rs currently uses a function-level attribute
#[cfg(all(target_os = "macos", target_arch = "aarch64"))] on
build_apple_intelligence_bridge(), which is evaluated for the build-script host;
instead remove that attribute and branch inside main() by reading Cargo's target
env vars (e.g., CARGO_CFG_TARGET_OS and CARGO_CFG_TARGET_ARCH or
CARGO_CFG_TARGET_ENV) from std::env to detect the target being built, then call
build_apple_intelligence_bridge() when those variables indicate macos + aarch64;
update any other similar guarded calls (lines calling
build_apple_intelligence_bridge or similar functions) to follow the same pattern
so the decision is made at runtime in build.rs rather than via #[cfg].
In `@tdn-desktop/src-tauri/swift/apple_intelligence.swift`:
- Around line 95-166: The detached Task created in processTextWithSystemPrompt
is never cancelled on semaphore timeout and maxTokens is unused; capture the
Task handle returned by Task.detached (or use Task.init) so you can call
task.cancel() if semaphore.wait times out, and ensure you propagate maxTokens
into the LanguageModelSession.respond calls (or remove the maxTokens parameter
from the bridge) — specifically update the Task detachment around Task.detached
{ ... } to assign it to a variable (e.g., let task = Task { ... }), call
task.cancel() when timeout == .timedOut before returning, and wire maxTokens
into session.respond (or drop the param) so the respond invocation(s) use the
token limit instead of being a no-op.
In `@tdn-desktop/src/components/quick-pane/QuickPaneApp.tsx`:
- Around line 276-337: Wrap the AI run in a session token to ignore late
completions: before calling commands.processQuickEntryText generate/capture a
local token (e.g., aiSessionToken = ++aiSessionCounter) and store it in
component state; after the await, verify the token still matches the current
aiSessionToken before applying any changes. When applying parsed results from
processQuickEntryText, explicitly assign every AI-managed field (use setTitle,
setBody, setShowBody, setStatus, setDue, setScheduled, setDeferUntil,
setProjectId, setAreaId) — do not only write truthy values; set fields to
null/undefined when parsed values are empty so stale values are cleared (follow
the shape in bindings.ts). Also ensure reset/dismiss paths increment/clear the
aiSessionCounter/token and setIsProcessingAI(false) so in-flight responses are
invalidated and ignored.
---
Nitpick comments:
In `@tdn-desktop/src-tauri/src/commands/vault.rs`:
- Around line 986-1093: Add tests mirroring the project-resolution tests to
cover area resolution paths: implement tests for create_task, create_project,
update_task, and update_project that create an Area via
manager.create_project/create_project flow (or create_area helper if available),
then call manager.create_task/CreateTaskOptions and
manager.update_task/TaskUpdate (and the project creation/update equivalents)
using area IDs; assert the resulting task.project/task.area (or project.area
fields) contain the area title (not the hash ID) and that unknown area IDs are
preserved verbatim. Target the manager methods create_task, create_project,
update_task, and update_project when adding these assertions to ensure area
ID-to-title resolution and unknown-ID preservation are covered.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d4dab3ec-0794-4870-b71a-b84f4a2169ba
📒 Files selected for processing (8)
tdn-desktop/src-tauri/build.rstdn-desktop/src-tauri/src/commands/ai.rstdn-desktop/src-tauri/src/commands/ai_resolve.rstdn-desktop/src-tauri/src/commands/mod.rstdn-desktop/src-tauri/src/commands/vault.rstdn-desktop/src-tauri/src/vault/manager.rstdn-desktop/src-tauri/swift/apple_intelligence.swifttdn-desktop/src/components/quick-pane/QuickPaneApp.tsx
✅ Files skipped from review due to trivial changes (1)
- tdn-desktop/src-tauri/src/commands/ai.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- tdn-desktop/src-tauri/src/commands/mod.rs
- tdn-desktop/src-tauri/src/commands/ai_resolve.rs
apple_intelligence.swift: - Replace manual JSON escaping with JSONSerialization for safe output - Cancel the detached Task on semaphore timeout (was left running) - Mark maxTokens as unused (_maxTokens) for ABI compatibility - Remove stale contentTagging comment QuickPaneApp.tsx: - Add session token (aiSessionRef) to ignore late AI completions when pane is dismissed and reopened during processing - Clear ALL fields on AI result (not just truthy ones) so re-processing doesn't leave stale values from a previous run Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary by CodeRabbit
New Features
Documentation
Tests