Conversation
WalkthroughThis PR applies wide-ranging refactors across frontend and backend: replaces the Rust YAML parser with serde_norway and IndexMap frontmatter, centralises domain types, introduces query-driven file loading, implements auto-save reliability (lastSaveTimestamp and forced saves), extracts multiple utilities, adds new hooks/components/tests, and updates documentation and scripts. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor as Editor Component
participant Store as EditorStore
participant Query as TanStack Query
participant Tauri as Tauri Backend
rect #CFE8FF
Note over User,Editor: File open (sync openFile + query load)
User->>Editor: request open file
Editor->>Store: openFile(file) (sync)
Store->>Query: hook subscribes (useEditorFileContent)
Query->>Tauri: fetch file content
Tauri-->>Query: MarkdownContent (frontmatter = IndexMap)
Query->>Store: update editorContent, frontmatter, imports
Store->>Editor: subscription triggers
Editor->>Editor: apply content programmatically (suppress listener)
end
rect #FFF2CF
Note over User,Store: Auto-save (debounce + forced save)
User->>Editor: types content
Editor->>Store: setEditorContent()
Store->>Store: scheduleAutoSave()
alt time since lastSaveTimestamp >= MAX
Store->>Tauri: immediate save (forced)
else
Store->>Store: schedule debounced save
end
Tauri-->>Store: save result
Store->>Store: set lastSaveTimestamp, isDirty=false
Store->>Query: invalidate fileContent & possibly directory queries
end
rect #E8FFE8
Note over Tauri,Store: External file change
Tauri->>Editor: emit 'file-changed' event
Editor->>Store: useFileChangeHandler sees event
alt isDirty = false and path matches
Store->>Query: invalidate fileContent query
Query->>Tauri: re-fetch updated content
else
Store->>Store: skip invalidation (preserve unsaved edits)
end
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120–150 minutes Specific areas to prioritise:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (4)src/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
{src/lib,src/hooks}/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/lib/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/lib/**/*.test.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-10-23T17:33:56.867ZApplied to files:
🧬 Code graph analysis (1)src/lib/schema.test.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (5)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src-tauri/src/commands/files.rs (1)
257-261: Bug: MDX imports are lost when updating frontmatter.
update_frontmatterrebuilds without preservingparsed.imports, dropping top-of-file imports.Apply this fix to keep imports:
- let new_content = rebuild_markdown_with_frontmatter(&frontmatter, &parsed.content)?; + let new_content = rebuild_markdown_with_frontmatter_and_imports( + &frontmatter, + &parsed.imports, + &parsed.content, + )?;Add a test that updates frontmatter on a file with imports and asserts imports remain intact.
🧹 Nitpick comments (7)
src/store/__tests__/editorStore.integration.test.ts (2)
17-17: Remove unused test variable to satisfy ESLint.
mockSaveFileis declared but never used. Delete it to clear@typescript-eslint/no-unused-vars.- let mockSaveFile: ReturnType<typeof vi.fn> + // (removed) unused @@ - // Create a mock for saveFile that we can spy on - mockSaveFile = vi.fn().mockResolvedValue(undefined) + // saveFile is spied per-test where neededAlso applies to: 23-25
20-22: Seed fake timers’ system time to avoid Date.now drift/negatives.Explicitly set a baseline time so max‑delay math is deterministic across environments.
beforeEach(() => { // Use fake timers for all tests - vi.useFakeTimers() + vi.useFakeTimers() + vi.setSystemTime(new Date(0)) @@ - // Setup: Last save was only 3 seconds ago + // Setup: Last save was only 3 seconds ago (relative to seeded time) @@ - // Setup: Last save was 11 seconds ago + // Setup: Last save was 11 seconds ago (relative to seeded time)Consider adding one assertion that advancing 2s after the final keystroke actually calls
saveFile(false)(e.g.,vi.advanceTimersByTime(2000)thenexpect(spy).toHaveBeenCalledWith(false)), to cover the debounce path end-to-end.Also applies to: 65-76, 85-90, 181-219
docs/tasks-todo/task-4-fix-file-watcher-race-condition.md (2)
484-484: Hyphenation nit.Use “high‑value feature.”
-**Recommended for v1.0.0**: - - Low risk, high value feature +**Recommended for v1.0.0**: + - Low risk, high-value feature
509-509: Use a proper heading instead of emphasis for section title.Converts emphasis to a markdown heading to satisfy MD036.
-**Total: 2.5 hours** +### Total: 2.5 hourssrc-tauri/src/commands/files.rs (1)
343-384: Simplify multi-line import parsing; remove stalelast_importcheck.
last_importis computed once and not updated inside the loop; logic still works but is harder to follow.- // Handle multi-line imports (lines that don't end with semicolon) - let last_import = imports.last().unwrap_or(&"").trim(); - while content_start_idx < lines.len() - && !last_import.ends_with(';') - && !last_import.ends_with("';") - && !last_import.ends_with("\";") - { - let current_line = lines[content_start_idx].trim(); + // Handle multi-line imports until a trailing semicolon line + while content_start_idx < lines.len() { + let current_line = lines[content_start_idx].trim(); if current_line.is_empty() { // Empty line might separate imports from content break; } else { // This is a continuation of the previous import imports.push(lines[content_start_idx]); content_start_idx += 1; - if current_line.ends_with(';') - || current_line.ends_with("';") - || current_line.ends_with("\";") - { + if current_line.ends_with(';') + || current_line.ends_with("';") + || current_line.ends_with("\";") { break; } } }docs/tasks-done/task-replace-custom-yaml-parser.md (2)
20-22: Replace bare URLs with autolinks (MD034).Use angle brackets to satisfy markdownlint.
-- **Crate**: https://crates.io/crates/serde_norway -- **Docs**: https://docs.rs/serde_norway/ +- **Crate**: <https://crates.io/crates/serde_norway> +- **Docs**: <https://docs.rs/serde_norway/> @@ -- serde_norway crate: https://crates.io/crates/serde_norway -- serde_norway docs: https://docs.rs/serde_norway/ +- serde_norway crate: <https://crates.io/crates/serde_norway> +- serde_norway docs: <https://docs.rs/serde_norway/>Also applies to: 488-489
321-356: Keep crate naming consistent in examples.Section header says “serde_yml” in test name; prefer “serde_norway” to match the migration.
-#[test] -fn test_serde_yml_handles_anchors() { +#[test] +fn test_serde_norway_handles_anchors() {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
.claude/settings.local.json(1 hunks)docs/developer/testing-guide.md(3 hunks)docs/reviews/analyysis-of-reviews.md(1 hunks)docs/tasks-done/task-add-integration-tests-for-reliability.md(1 hunks)docs/tasks-done/task-fix-auto-save-data-loss-risk.md(3 hunks)docs/tasks-done/task-replace-custom-yaml-parser.md(1 hunks)docs/tasks-todo/task-2-replace-custom-yaml-parser.md(0 hunks)docs/tasks-todo/task-4-fix-file-watcher-race-condition.md(1 hunks)docs/tasks-todo/task-5-improve-rust-test-suite.md(1 hunks)docs/tasks-todo/task-6-centralize-frontend-types.md(1 hunks)src-tauri/Cargo.toml(1 hunks)src-tauri/src/commands/files.rs(16 hunks)src-tauri/src/commands/project.rs(1 hunks)src-tauri/src/models/file_entry.rs(7 hunks)src/hooks/editor/useEditorHandlers.test.ts(2 hunks)src/lib/project-registry/index.test.ts(1 hunks)src/store/__tests__/editorStore.integration.test.ts(1 hunks)src/store/__tests__/storeQueryIntegration.test.ts(1 hunks)src/store/editorStore.ts(5 hunks)src/test/mocks/toast.ts(1 hunks)src/test/utils/integration-helpers.ts(1 hunks)
💤 Files with no reviewable changes (1)
- docs/tasks-todo/task-2-replace-custom-yaml-parser.md
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: Do not use React Hook Form; prefer Direct Store Pattern to avoid infinite loops
Use react-hotkeys-hook for keyboard shortcuts (e.g., mod+s, mod+1)
Type store destructuring explicitly when using Zustand stores
Files:
src/store/__tests__/editorStore.integration.test.tssrc/hooks/editor/useEditorHandlers.test.tssrc/store/editorStore.tssrc/test/utils/integration-helpers.tssrc/lib/project-registry/index.test.tssrc/store/__tests__/storeQueryIntegration.test.tssrc/test/mocks/toast.ts
src/store/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
In stores (no React hooks), dispatch DOM CustomEvents for cross-layer communication (Bridge Pattern)
Files:
src/store/__tests__/editorStore.integration.test.tssrc/store/editorStore.tssrc/store/__tests__/storeQueryIntegration.test.ts
src-tauri/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use modern Rust formatting: format("{variable}")
Files:
src-tauri/src/models/file_entry.rssrc-tauri/src/commands/project.rssrc-tauri/src/commands/files.rs
{src/lib,src/hooks}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Invalidate TanStack Query caches on mutation success using the correct queryKey
Files:
src/hooks/editor/useEditorHandlers.test.tssrc/lib/project-registry/index.test.ts
src/hooks/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place reusable React hooks and side-effect logic in src/hooks/
Files:
src/hooks/editor/useEditorHandlers.test.ts
src/hooks/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Write integration tests for hooks and workflows under src/hooks/
Files:
src/hooks/editor/useEditorHandlers.test.ts
src/lib/**
📄 CodeRabbit inference engine (CLAUDE.md)
Extract complex, testable business logic (50+ lines or 2+ components) into src/lib/ modules
Files:
src/lib/project-registry/index.test.ts
src/lib/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Write unit tests for modules in src/lib/
Files:
src/lib/project-registry/index.test.ts
docs/developer/{performance-guide,testing-guide}.md
📄 CodeRabbit inference engine (CLAUDE.md)
Consult core guides: performance-guide.md and testing-guide.md during daily development
Files:
docs/developer/testing-guide.md
docs/developer/**
📄 CodeRabbit inference engine (CLAUDE.md)
Update docs/developer/ guides when introducing new patterns
Files:
docs/developer/testing-guide.md
🧠 Learnings (3)
📚 Learning: 2025-10-23T17:33:56.849Z
Learnt from: CR
PR: dannysmith/astro-editor#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.849Z
Learning: Applies to src/lib/**/*.test.{ts,tsx} : Write unit tests for modules in src/lib/
Applied to files:
docs/developer/testing-guide.md
📚 Learning: 2025-10-23T17:33:56.849Z
Learnt from: CR
PR: dannysmith/astro-editor#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.849Z
Learning: Applies to src/components/**/*.test.{ts,tsx} : Write component tests for user interactions under src/components/
Applied to files:
docs/developer/testing-guide.md
📚 Learning: 2025-10-23T17:33:56.849Z
Learnt from: CR
PR: dannysmith/astro-editor#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.849Z
Learning: Applies to src/hooks/**/*.test.{ts,tsx} : Write integration tests for hooks and workflows under src/hooks/
Applied to files:
docs/developer/testing-guide.md
🧬 Code graph analysis (4)
src/store/__tests__/editorStore.integration.test.ts (3)
src/store/editorStore.ts (1)
useEditorStore(222-507)src/store/projectStore.ts (1)
useProjectStore(46-428)src/test/mocks/toast.ts (1)
resetToastMocks(18-23)
src/test/utils/integration-helpers.ts (1)
src/lib/query-client.ts (1)
queryClient(4-18)
src/store/__tests__/storeQueryIntegration.test.ts (3)
src/store/editorStore.ts (1)
useEditorStore(222-507)src/store/projectStore.ts (1)
useProjectStore(46-428)src/test/mocks/toast.ts (1)
resetToastMocks(18-23)
src-tauri/src/commands/files.rs (1)
src-tauri/src/models/file_entry.rs (1)
new(20-70)
🪛 Biome (2.1.2)
src/test/utils/integration-helpers.ts
[error] 24-24: expected > but instead found client
Remove client
(parse)
[error] 24-24: Invalid assignment to <QueryClientProvider client
This expression cannot be assigned to
(parse)
[error] 26-26: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
🪛 ESLint
src/store/__tests__/editorStore.integration.test.ts
[error] 17-17: 'mockSaveFile' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
🪛 LanguageTool
docs/tasks-todo/task-4-fix-file-watcher-race-condition.md
[grammar] ~484-~484: Use a hyphen to join words.
Context: ...commended for v1.0.0**: - Low risk, high value feature - Completes file watcher s...
(QB_NEW_EN_HYPHEN)
docs/reviews/analyysis-of-reviews.md
[style] ~117-~117: Consider using a different verb for a more formal wording.
Context: ...g it improves maintainability but won't fix any user-facing problems. Action: Defe...
(FIX_RESOLVE)
docs/tasks-todo/task-5-improve-rust-test-suite.md
[style] ~14-~14: To elevate your writing, try using a synonym here.
Context: ...al I/O makes timing-dependent scenarios hard to reproduce **Evidence from Rust Test...
(HARD_TO)
🪛 markdownlint-cli2 (0.18.1)
docs/tasks-todo/task-6-centralize-frontend-types.md
481-481: Bare URL used
(MD034, no-bare-urls)
docs/tasks-todo/task-4-fix-file-watcher-race-condition.md
509-509: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
docs/reviews/analyysis-of-reviews.md
150-150: Reversed link syntax
('T')[0]
(MD011, no-reversed-links)
docs/tasks-todo/task-5-improve-rust-test-suite.md
397-397: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/tasks-done/task-add-integration-tests-for-reliability.md
78-78: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
100-100: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
108-108: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
128-128: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
136-136: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
144-144: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
156-156: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
169-169: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
184-184: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
367-367: Bare URL used
(MD034, no-bare-urls)
docs/tasks-done/task-replace-custom-yaml-parser.md
20-20: Bare URL used
(MD034, no-bare-urls)
21-21: Bare URL used
(MD034, no-bare-urls)
22-22: Bare URL used
(MD034, no-bare-urls)
488-488: Bare URL used
(MD034, no-bare-urls)
489-489: Bare URL used
(MD034, no-bare-urls)
docs/developer/testing-guide.md
26-26: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
52-52: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
85-85: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
106-106: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (24)
src/lib/project-registry/index.test.ts (1)
8-14: LGTM! Cleaner import paths.The refactored unmock calls and imports now use barrel exports and local relative paths instead of deep relative paths, improving readability and maintainability.
docs/reviews/analyysis-of-reviews.md (1)
1-259: Excellent meta-analysis document.This document provides valuable prioritization and clear separation of real risks from architectural preferences. The structured approach to pre-1.0.0 vs. post-1.0.0 tasks is particularly helpful.
docs/tasks-done/task-add-integration-tests-for-reliability.md (1)
1-391: Comprehensive integration test plan.The phased approach is pragmatic, with Phase 1 focused on auto-save validation and Phase 2 deferred for file watcher tests. The test scenarios are detailed and actionable.
docs/developer/testing-guide.md (1)
18-125: Excellent test organization guidance.The three-tier structure (Unit, Integration, Test Infrastructure) is clear and industry-standard. The decision tree and anti-patterns section will help developers place tests correctly.
docs/tasks-done/task-fix-auto-save-data-loss-risk.md (1)
1-175: Sound approach to fixing auto-save data loss.The implementation using
lastSaveTimestampand a maximum 10-second delay addresses the critical risk while preserving the debounce behavior for normal typing.docs/tasks-todo/task-5-improve-rust-test-suite.md (1)
1-493: Well-structured Rust test improvement plan.The filesystem trait abstraction and in-memory implementation follow best practices. The comprehensive negative and edge case examples will significantly improve test coverage and reliability.
docs/tasks-todo/task-6-centralize-frontend-types.md (1)
1-500: Solid type centralization plan.The phased approach and clear migration strategy will help prevent type drift while minimizing the risk of breaking changes. The example type definitions with JSDoc are well-structured.
src/test/utils/integration-helpers.ts (4)
10-35: Good test setup function.The
setupEditorIntegrationTestcreates a clean QueryClient with retries disabled and no cache, appropriate for integration tests. The Tauri mock reset ensures test isolation.
45-59: Well-designed typing simulator.The
simulateContinuousTypingfunction correctly uses real timers andact()to simulate typing behavior, making it suitable for testing auto-save debouncing and timing logic.
65-67: Simple and useful timer utility.The
advanceTimersByTimefunction provides a consistent API for fake timer tests.
77-90: Useful polling utility.The
waitForConditionfunction provides a clean way to wait for async conditions in integration tests, with configurable timeout and polling interval.src/hooks/editor/useEditorHandlers.test.ts (1)
38-38: LGTM! Test mocks correctly updated for new timestamp field.The addition of
lastSaveTimestampto the mock state aligns with the new auto-save max-delay feature introduced in the editor store.Also applies to: 68-68
.claude/settings.local.json (1)
51-53: LGTM! Appropriate domains for Rust dependency management.The addition of rustsec.org and crates.io enables security advisory checks and package information lookups for the new Rust dependencies.
src-tauri/src/commands/project.rs (1)
458-460: LGTM! Correctly migrated to IndexMap for ordered frontmatter.The conversion from JSON object to IndexMap aligns with the broader migration to ordered frontmatter handling and matches the updated FileEntry type signature.
src/test/mocks/toast.ts (1)
1-28: LGTM! Clean and well-documented test utility.The mock implementation follows standard Vitest patterns and provides a clear API for testing toast notifications. The reset function properly clears all mock state between tests.
src/store/__tests__/storeQueryIntegration.test.ts (1)
1-215: LGTM! Comprehensive integration test coverage.The test suite effectively covers critical store-query integration scenarios including dirty state management, file switching, and frontmatter updates. The test structure is clear and properly isolates each scenario.
src-tauri/src/models/file_entry.rs (1)
1-1: LGTM! Consistent migration to IndexMap for ordered frontmatter.The type changes from HashMap to IndexMap for the frontmatter field and with_frontmatter method align with the broader PR objective to maintain deterministic frontmatter ordering. All tests are correctly updated to use IndexMap::new().
Also applies to: 16-16, 72-72
src/store/editorStore.ts (4)
10-10: LGTM! Appropriate max-delay constant for data loss prevention.The 10-second maximum delay between auto-saves provides a good balance between preventing data loss and avoiding excessive save operations.
209-209: LGTM! Proper timestamp tracking initialization.The lastSaveTimestamp field is correctly added to the state interface and initialized to null, allowing the max-delay logic to distinguish between "never saved" and "saved at timestamp."
Also applies to: 232-232
379-379: LGTM! Timestamp updated on successful save.Setting lastSaveTimestamp after a successful save enables the max-delay forced save mechanism to track elapsed time since the last save.
465-474: LGTM! Max-delay forced save logic is sound.The implementation correctly:
- Calculates time elapsed since last save
- Forces an immediate save (without toast) when max delay is exceeded
- Returns early to bypass the normal debounce mechanism
- Only applies when file is dirty and has been saved at least once
This prevents potential data loss during prolonged editing sessions while preserving the normal debounced auto-save behavior for recent changes.
src-tauri/Cargo.toml (1)
26-26: serde_norway dependency verified as current and secure.Version 0.9.42 is the latest available on crates.io with no known security vulnerabilities.
src-tauri/src/commands/files.rs (2)
150-225: Clarify policy: allow copying from arbitrary source paths?
copy_file_to_assets(_with_override)copies fromsource_pathwithout validation. If users can pass arbitrary paths, this enables copying any file on the system into the project. Likely fine for user‑initiated “Import asset…” flows; if not intended, validatesource_pathsimilarly or restrict to OS file‑picker outputs.If restriction is desired, consider:
- // Copy the file - fs::copy(&source_path, &validated_final_path) + // Optional: validate/whitelist source before copying + fs::copy(&source_path, &validated_final_path) .map_err(|e| format!("Failed to copy file: {e}"))?;
511-567: Nice serialization touch.Ordered frontmatter + recursive date normalization with
serde_norwaykeeps output deterministic and tidy. The “exactly one trailing newline” guard is a good UX detail.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/test/utils/integration-helpers.tsx (3)
43-57: Timer system mismatch with integration tests.This function uses real timers (
setTimeouton line 55) while the integration tests use fake timers (vi.useFakeTimers()). This creates a conflict—when fake timers are active,setTimeoutwon't advance withoutvi.advanceTimersByTime().Consider two approaches:
Option 1: Use fake timers with explicit advances
export async function simulateContinuousTyping( setContent: (content: string) => void, durationMs: number, intervalMs: number ): Promise<void> { const iterations = Math.floor(durationMs / intervalMs) for (let i = 0; i < iterations; i++) { act(() => { setContent(`content ${i}`) }) - // Use real timers to test actual auto-save timing - await new Promise(resolve => setTimeout(resolve, intervalMs)) + // Advance fake timers if they're in use + vi.advanceTimersByTime(intervalMs) } }Option 2: Document that this helper requires real timers
/** * Simulates continuous typing for integration tests * Calls setContent repeatedly with a specified interval + * + * @note This function uses real timers and should not be used with vi.useFakeTimers() * * @param setContent - Function to set editor content * @param durationMs - Total duration to simulate typing (ms) * @param intervalMs - Interval between keystrokes (ms) */
63-65: Consider removing unnecessary wrapper.This function is a thin wrapper that adds no value—it simply calls
vi.advanceTimersByTime(ms). The integration test file already importsvidirectly and callsvi.advanceTimersByTime()at lines 87, 106, and 109, creating inconsistency.Consider removing this wrapper and using
vi.advanceTimersByTime()directly throughout the codebase, or enhance this wrapper to provide additional test-specific behavior if needed.
75-88: Timer system mismatch with integration tests.Similar to
simulateContinuousTyping, this function uses real timers (setTimeouton line 86) while the integration tests use fake timers. When fake timers are active, this polling loop won't work as expected since the timer won't advance.If this helper is intended for use with fake timers, consider using
vi.advanceTimersByTime()or document that it requires real timers:/** * Waits for a condition to be true, checking at specified intervals * Useful for waiting for async operations in integration tests + * + * @note This function uses real timers and should not be used with vi.useFakeTimers() + * For fake timer tests, check conditions synchronously after vi.advanceTimersByTime() * * @param condition - Function that returns true when condition is met * @param timeout - Maximum time to wait (ms) * @param interval - How often to check the condition (ms) */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src-tauri/src/commands/files.rs(15 hunks)src/store/__tests__/editorStore.integration.test.ts(1 hunks)src/store/__tests__/storeQueryIntegration.test.ts(1 hunks)src/test/utils/integration-helpers.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/store/tests/storeQueryIntegration.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
src-tauri/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use modern Rust formatting: format("{variable}")
Files:
src-tauri/src/commands/files.rs
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: Do not use React Hook Form; prefer Direct Store Pattern to avoid infinite loops
Use react-hotkeys-hook for keyboard shortcuts (e.g., mod+s, mod+1)
Type store destructuring explicitly when using Zustand stores
Files:
src/test/utils/integration-helpers.tsxsrc/store/__tests__/editorStore.integration.test.ts
src/store/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
In stores (no React hooks), dispatch DOM CustomEvents for cross-layer communication (Bridge Pattern)
Files:
src/store/__tests__/editorStore.integration.test.ts
🧠 Learnings (4)
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/hooks/**/*.test.{ts,tsx} : Write integration tests for hooks and workflows under src/hooks/
Applied to files:
src/test/utils/integration-helpers.tsxsrc/store/__tests__/editorStore.integration.test.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/components/**/*.test.{ts,tsx} : Write component tests for user interactions under src/components/
Applied to files:
src/test/utils/integration-helpers.tsxsrc/store/__tests__/editorStore.integration.test.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to {src/lib,src/hooks}/**/*.{ts,tsx} : Invalidate TanStack Query caches on mutation success using the correct queryKey
Applied to files:
src/test/utils/integration-helpers.tsx
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/lib/**/*.test.{ts,tsx} : Write unit tests for modules in src/lib/
Applied to files:
src/test/utils/integration-helpers.tsx
🧬 Code graph analysis (3)
src-tauri/src/commands/files.rs (1)
src-tauri/src/commands/mdx_components.rs (1)
validate_project_path(17-32)
src/test/utils/integration-helpers.tsx (1)
src/lib/query-client.ts (1)
queryClient(4-18)
src/store/__tests__/editorStore.integration.test.ts (3)
src/store/editorStore.ts (1)
useEditorStore(222-507)src/store/projectStore.ts (1)
useProjectStore(46-428)src/test/mocks/toast.ts (1)
resetToastMocks(18-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (6)
src/store/__tests__/editorStore.integration.test.ts (5)
1-13: LGTM!The imports and mock setup are appropriate for the integration test suite. The mock FileEntry includes all necessary fields for testing editor behavior.
15-69: LGTM!The test setup is thorough and follows best practices:
- Fake timers with deterministic baseline time
- Complete state reset for both stores
- Proper cleanup in afterEach
71-120: LGTM!The force save tests properly validate the 10-second max delay logic and debounced save behavior. The spy pattern is used correctly to verify save invocations.
122-191: LGTM!The tests comprehensively cover dirty state transitions and auto-save scheduling behavior. The timeout clearing test is particularly good for preventing memory leaks.
193-235: LGTM!The max delay force save logic tests properly validate the 10-second threshold behavior with appropriate spy usage and state setup.
src/test/utils/integration-helpers.tsx (1)
10-33: LGTM!The
setupEditorIntegrationTestfunction provides a clean test environment with appropriate QueryClient configuration (no retries, no cache) and proper mock reset. The wrapper pattern follows React Query testing best practices.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/store/__tests__/storeQueryIntegration.test.ts (1)
1-209: Critical test still missing: MDX imports preservation during frontmatter updates.The test suite lacks coverage for the core bug fix mentioned in the PR objectives: "Address reported bug where MDX/top-of-file imports are lost when updating frontmatter." A test that updates frontmatter on a file with imports and verifies imports remain intact is essential to validate the fix.
Refer to the previous review comment that provided a test template for this scenario.
src/store/__tests__/editorStore.integration.test.ts (1)
1-235: Missing critical test: imports preservation during save operations.While the auto-save tests are comprehensive, this file lacks the critical test requested in PR objectives: verifying that imports are preserved when frontmatter is updated and saved. This test would validate the fix for the reported bug where "MDX/top-of-file imports are lost when updating frontmatter."
The test should verify that
save_markdown_contentis called with theimportsparameter when frontmatter is updated on a file that has imports. Refer to the previous review comment for a test template.
🧹 Nitpick comments (2)
docs/tasks-todo/task-4-fix-file-watcher-race-condition.md (2)
98-98: Markdown linting: Add language identifier to fenced code blocks.Fenced code blocks at lines 98 and 815 are missing language specifiers. This helps with syntax highlighting in documentation viewers.
Apply these diffs:
-``` +```typescript ┌─────────────────┐And:
-``` +```typescript // While typing: isDirty=trueAlso applies to: 815-815
163-225: Document alignment with learnings: Emphasize cache invalidation pattern.The
useEditorFileContenthook design is solid. However, given the retrieved learning about "Invalidate TanStack Query caches on mutation success using the correct queryKey," ensure the documentation explicitly calls out that Phase 2's cache invalidation is the critical coupling point between this hook and the save flow.Add a callout near line 181 clarifying that this hook depends on the save flow invalidating
queryKeys.fileContent(...)correctly, so consider cross-referencing Phase 2 (lines 333–406) here to make the dependency explicit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
docs/tasks-todo/task-4-fix-file-watcher-race-condition.md(1 hunks)src/components/editor/Editor.tsx(3 hunks)src/components/layout/Layout.tsx(2 hunks)src/components/layout/LeftSidebar.tsx(1 hunks)src/hooks/editor/useEditorHandlers.test.ts(2 hunks)src/hooks/useCreateFile.ts(1 hunks)src/hooks/useEditorFileContent.ts(1 hunks)src/hooks/useFileChangeHandler.ts(1 hunks)src/lib/commands/app-commands.ts(1 hunks)src/store/__tests__/editorStore.integration.test.ts(1 hunks)src/store/__tests__/storeQueryIntegration.test.ts(1 hunks)src/store/editorStore.ts(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/hooks/editor/useEditorHandlers.test.ts
🧰 Additional context used
📓 Path-based instructions (11)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: Do not use React Hook Form; prefer Direct Store Pattern to avoid infinite loops
Use react-hotkeys-hook for keyboard shortcuts (e.g., mod+s, mod+1)
Type store destructuring explicitly when using Zustand stores
Files:
src/hooks/useCreateFile.tssrc/components/layout/Layout.tsxsrc/hooks/useFileChangeHandler.tssrc/hooks/useEditorFileContent.tssrc/store/__tests__/storeQueryIntegration.test.tssrc/store/editorStore.tssrc/lib/commands/app-commands.tssrc/components/editor/Editor.tsxsrc/components/layout/LeftSidebar.tsxsrc/store/__tests__/editorStore.integration.test.ts
{src/lib,src/hooks}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Invalidate TanStack Query caches on mutation success using the correct queryKey
Files:
src/hooks/useCreateFile.tssrc/hooks/useFileChangeHandler.tssrc/hooks/useEditorFileContent.tssrc/lib/commands/app-commands.ts
src/hooks/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place reusable React hooks and side-effect logic in src/hooks/
Files:
src/hooks/useCreateFile.tssrc/hooks/useFileChangeHandler.tssrc/hooks/useEditorFileContent.ts
src/components/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/components/**/*.tsx: Components should access stores directly (Direct Store Pattern) instead of prop callbacks that cause dependency loops
Extract helper components when JSX is repeated 3+ times
Use AutoExpandingTextarea instead of CSS field-sizing: content in WebKit
Files:
src/components/layout/Layout.tsxsrc/components/editor/Editor.tsxsrc/components/layout/LeftSidebar.tsx
src/components/layout/Layout.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/components/layout/Layout.tsx: In Layout, subscribe to dispatched CustomEvents and use Query Client cache for context data (Bridge Pattern)
Add event listeners related to command palette behavior in Layout.tsx when needed
Files:
src/components/layout/Layout.tsx
src/components/**/[A-Z]*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Name React component files in PascalCase within src/components
Files:
src/components/layout/Layout.tsxsrc/components/editor/Editor.tsxsrc/components/layout/LeftSidebar.tsx
src/components/[a-z0-9-]*/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Use kebab-case for component directory names under src/components
Files:
src/components/layout/Layout.tsxsrc/components/editor/Editor.tsxsrc/components/layout/LeftSidebar.tsx
src/store/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
In stores (no React hooks), dispatch DOM CustomEvents for cross-layer communication (Bridge Pattern)
Files:
src/store/__tests__/storeQueryIntegration.test.tssrc/store/editorStore.tssrc/store/__tests__/editorStore.integration.test.ts
src/lib/commands/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use the Command Pattern via globalCommandRegistry for editor operations
Files:
src/lib/commands/app-commands.ts
src/lib/**
📄 CodeRabbit inference engine (CLAUDE.md)
Extract complex, testable business logic (50+ lines or 2+ components) into src/lib/ modules
Files:
src/lib/commands/app-commands.ts
src/lib/commands/app-commands.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Define app-level commands in src/lib/commands/app-commands.ts
Files:
src/lib/commands/app-commands.ts
🧠 Learnings (13)
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/components/layout/Layout.tsx : Add event listeners related to command palette behavior in Layout.tsx when needed
Applied to files:
src/components/layout/Layout.tsx
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/components/layout/Layout.tsx : In Layout, subscribe to dispatched CustomEvents and use Query Client cache for context data (Bridge Pattern)
Applied to files:
src/components/layout/Layout.tsx
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/hooks/**/*.ts : Place reusable React hooks and side-effect logic in src/hooks/
Applied to files:
src/hooks/useFileChangeHandler.tssrc/hooks/useEditorFileContent.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/**/*.{ts,tsx} : Use react-hotkeys-hook for keyboard shortcuts (e.g., mod+s, mod+1)
Applied to files:
src/hooks/useFileChangeHandler.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to {src/lib,src/hooks}/**/*.{ts,tsx} : Invalidate TanStack Query caches on mutation success using the correct queryKey
Applied to files:
src/hooks/useFileChangeHandler.tssrc/hooks/useEditorFileContent.tssrc/store/__tests__/storeQueryIntegration.test.tsdocs/tasks-todo/task-4-fix-file-watcher-race-condition.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Use TanStack Query for server state, Zustand for client state, and local state for UI-only concerns
Applied to files:
src/hooks/useEditorFileContent.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/hooks/**/*.test.{ts,tsx} : Write integration tests for hooks and workflows under src/hooks/
Applied to files:
src/store/__tests__/storeQueryIntegration.test.tsdocs/tasks-todo/task-4-fix-file-watcher-race-condition.mdsrc/store/__tests__/editorStore.integration.test.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/components/**/*.test.{ts,tsx} : Write component tests for user interactions under src/components/
Applied to files:
src/store/__tests__/storeQueryIntegration.test.tssrc/store/__tests__/editorStore.integration.test.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/lib/**/*.test.{ts,tsx} : Write unit tests for modules in src/lib/
Applied to files:
src/store/__tests__/storeQueryIntegration.test.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Use memoization, debouncing (2s auto-save), and getState() to prevent render cascades
Applied to files:
src/store/editorStore.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/lib/commands/app-commands.ts : Define app-level commands in src/lib/commands/app-commands.ts
Applied to files:
src/lib/commands/app-commands.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/lib/commands/command-context.ts : Add or update command context functions in src/lib/commands/command-context.ts
Applied to files:
src/lib/commands/app-commands.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/lib/commands/**/*.ts : Use the Command Pattern via globalCommandRegistry for editor operations
Applied to files:
src/lib/commands/app-commands.ts
🧬 Code graph analysis (7)
src/components/layout/Layout.tsx (2)
src/hooks/useEditorFileContent.ts (1)
useEditorFileContent(15-58)src/hooks/useFileChangeHandler.ts (1)
useFileChangeHandler(21-57)
src/hooks/useFileChangeHandler.ts (4)
src/store/editorStore.ts (1)
useEditorStore(221-474)src/store/projectStore.ts (1)
useProjectStore(46-428)src/lib/query-client.ts (1)
queryClient(4-18)src/lib/query-keys.ts (1)
queryKeys(3-40)
src/hooks/useEditorFileContent.ts (3)
src/store/editorStore.ts (1)
useEditorStore(221-474)src/store/projectStore.ts (1)
useProjectStore(46-428)src/hooks/queries/useFileContentQuery.ts (1)
useFileContentQuery(24-37)
src/store/__tests__/storeQueryIntegration.test.ts (2)
src/store/editorStore.ts (2)
FileEntry(177-186)useEditorStore(221-474)src/test/mocks/toast.ts (1)
resetToastMocks(18-23)
src/store/editorStore.ts (2)
src/lib/query-client.ts (1)
queryClient(4-18)src/lib/query-keys.ts (1)
queryKeys(3-40)
src/components/editor/Editor.tsx (1)
src/store/editorStore.ts (1)
useEditorStore(221-474)
src/store/__tests__/editorStore.integration.test.ts (3)
src/store/editorStore.ts (2)
FileEntry(177-186)useEditorStore(221-474)src/store/projectStore.ts (1)
useProjectStore(46-428)src/test/mocks/toast.ts (1)
resetToastMocks(18-23)
🪛 LanguageTool
docs/tasks-todo/task-4-fix-file-watcher-race-condition.md
[style] ~12-~12: ‘on top of that’ might be wordy. Consider a shorter alternative.
Context: ... implements file watcher event handling on top of that architecture. ### Discovery The archi...
(EN_WORDINESS_PREMIUM_ON_TOP_OF_THAT)
[style] ~398-~398: Consider an alternative to strengthen your wording.
Context: ...lidation. Alternative (cleaner but more changes): Create hooks/useEditorSave.ts that ...
(CHANGES_ADJUSTMENTS)
[style] ~675-~675: To form a complete sentence, be sure to include a subject.
Context: ... v1.0.0 timeline: 4-5 hours total. Can be done in single session. --- ## Suc...
(MISSING_IT_THERE)
🪛 markdownlint-cli2 (0.18.1)
docs/tasks-todo/task-4-fix-file-watcher-race-condition.md
98-98: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
815-815: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (14)
docs/tasks-todo/task-4-fix-file-watcher-race-condition.md (1)
243-307: Verify Editor subscription performance claim.Lines 309–313 justify subscribing to
editorContent(which changes on every keystroke) by noting the string comparison guard and claiming it's <1ms. While the guard is valid, make sure this is validated with profiling data during implementation (Phase 4, lines 503–509).During Phase 4 manual testing (step 6, lines 879–883), confirm via React DevTools Profiler that:
- The Editor component itself does NOT re-render on every keystroke (only the CodeMirror view updates via dispatch)
- String comparison cost is indeed negligible
If profiling shows unexpected overhead, consider memoizing the editorContent selector or using a custom equality check.
src/hooks/useCreateFile.ts (1)
208-208: LGTM! Fire-and-forget pattern aligns with new architecture.The removal of
awaitis correct sinceopenFileis synchronous (only updates state and dispatches events). Content loading is now handled separately by theuseEditorFileContenthook, which properly decouples file opening from content fetching.src/lib/commands/app-commands.ts (1)
454-458: LGTM! Simplified command execution.Removing
async/awaitis correct sinceopenFileis synchronous. This simplifies the search command execution and aligns with the fire-and-forget pattern used throughout the codebase.Based on learnings
src/components/layout/LeftSidebar.tsx (1)
213-213: LGTM! Minor cleanup.Removing the
voidoperator is a stylistic improvement with no functional impact.src/components/layout/Layout.tsx (1)
16-17: LGTM! Clean integration of new file-loading architecture.The addition of
useEditorFileContentanduseFileChangeHandlerproperly implements the Bridge Pattern for cross-layer communication. These hooks centralize query-based file loading and external change detection, improving the separation of concerns.As per coding guidelines
Also applies to: 32-36
src/components/editor/Editor.tsx (2)
29-29: LGTM! Enables external content synchronization.Adding
editorContentfrom the store allows the editor to react to external content changes (e.g., from file reloads viauseEditorFileContent).
230-252: LGTM! Proper content synchronization with external changes.The updated effect correctly handles both file switches and external content changes. The
isProgrammaticUpdateguard prevents infinite loops, and the content difference check avoids unnecessary updates. Including bothcurrentFileIdandeditorContentin the dependency array is necessary to support the new query-based file loading architecture.src/hooks/useEditorFileContent.ts (3)
21-30: LGTM! Proper stale content prevention.Immediately clearing editor state when the file changes prevents showing stale content from the previous file during query loading. The conditional check ensures this only runs when opening a file, not when closing.
33-36: LGTM! Proper query setup.The query correctly fetches content based on the current file and project path, with appropriate null handling.
39-55: LGTM! Critical isDirty protection implemented correctly.The effect properly bridges server state (query data) with local editing state. The
isDirtycheck is crucial—it prevents overwriting user edits when query data arrives. This ensures the user's in-progress changes always take precedence over server data.src/hooks/useFileChangeHandler.ts (1)
21-57: LGTM! Clean implementation of external file-change handling.The hook correctly:
- Uses the Direct Store Pattern with
getState()to avoid unnecessary re-renders- Respects
isDirtystate to preserve unsaved user edits- Invalidates the query using the correct queryKey that matches the pattern in
saveFile- Properly cleans up the event listener on unmount
src/store/editorStore.ts (3)
233-248: Architectural shift to deferred content loading looks good.The synchronous
openFilenow correctly delegates content loading to theuseEditorFileContenthook, maintaining clean separation between state management (store) and data fetching (query hooks). The Bridge Pattern event dispatch aligns with the coding guidelines for cross-layer communication.
344-364: Query invalidation properly updated for new architecture.The invalidation logic correctly:
- Updates
lastSaveTimestampfor max-delay tracking- Invalidates file content using the same queryKey as
useFileChangeHandler- Invalidates directory queries for the collection using prefix matching
As per coding guidelines for TanStack Query cache invalidation.
430-441: Max-delay implementation is correct with documented edge case.The max-delay logic properly forces a save when
timeSinceLastSave >= MAX_AUTO_SAVE_DELAY_MS, preventing unbounded auto-save delays during continuous editing.Edge case note: If
lastSaveTimestampisnull(e.g., unsaved new file or after failed first save), the max-delay check won't trigger. This is acceptable because:
- The first auto-save uses the normal debounce delay (2s default)
- Failed saves trigger recovery data backup (lines 379-384) and error notifications
- Subsequent successful saves will enable max-delay tracking
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/store/__tests__/editorStore.integration.test.ts (1)
267-298: Critical: Test doesn't validate the frontmatter update workflow.The test title claims to test "updating frontmatter and saving," but lines 287-288 explicitly state it skips calling
updateFrontmatterField. This doesn't fulfill the PR objective requirement to "add a test that updates frontmatter on a file with imports and asserts imports remain intact."The test should actually call
updateFrontmatterFieldto validate the full workflow that had the bug.Update the test to actually update frontmatter before saving:
it('should preserve imports when updating frontmatter and saving', async () => { // Setup: File with imports const mockImports = "import { Component } from 'astro:components';" useEditorStore.setState({ currentFile: mockFileEntry, imports: mockImports, frontmatter: { title: 'Original Title' }, editorContent: '# Content', - isDirty: true, // Mark as dirty so saveFile actually saves + isDirty: false, lastSaveTimestamp: Date.now(), autoSaveTimeoutId: null, }) useProjectStore.setState({ projectPath: '/test', }) const store = useEditorStore.getState() - // Don't call updateFrontmatterField since it schedules auto-save - // Just directly call saveFile + // Update frontmatter (this should set isDirty and schedule auto-save) + store.updateFrontmatterField('title', 'New Title') + + // Save manually (don't wait for auto-save) await store.saveFile(false) // Verify: imports parameter was passed to save_markdown_content expect(globalThis.mockTauri.invoke).toHaveBeenCalledWith( 'save_markdown_content', expect.objectContaining({ imports: mockImports, }) ) })
🧹 Nitpick comments (2)
src/store/__tests__/editorStore.integration.test.ts (1)
300-334: Consider testing the actual content editing workflow.Similar to the previous test, this test sets
editorContentdirectly viasetStateinstead of callingsetEditorContent(). While this verifies thatsaveFilepreserves imports, it doesn't validate the full editing workflow.Update to test the actual store action:
it('should preserve imports when editing content and saving', async () => { // Setup: File with complex multiline imports const mockImports = `import { Component } from 'astro:components'; import { Foo, Bar } from './utils';` useEditorStore.setState({ currentFile: mockFileEntry, imports: mockImports, frontmatter: { title: 'Test' }, - editorContent: 'Updated content', - isDirty: true, // Mark as dirty so saveFile actually saves + editorContent: 'Original content', + isDirty: false, lastSaveTimestamp: Date.now(), autoSaveTimeoutId: null, }) useProjectStore.setState({ projectPath: '/test', }) const store = useEditorStore.getState() - // Trigger save directly + // Edit content through store action + store.setEditorContent('Updated content') + + // Save manually await store.saveFile(false) // Verify: imports were preserved expect(globalThis.mockTauri.invoke).toHaveBeenCalledWith( 'save_markdown_content', expect.objectContaining({ imports: mockImports, }) ) })src-tauri/src/commands/files.rs (1)
126-131: Use modern Rust formatting.Line 128 uses older
format!syntax. Based on coding guidelines.Apply this diff:
// Reconstruct with extension if present if let Some(ext) = extension { - format!("{}.{}", kebab_filename, ext.to_lowercase()) + let ext_lower = ext.to_lowercase(); + format!("{kebab_filename}.{ext_lower}") } else {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/tasks-todo/task-5-improve-rust-test-suite.md(1 hunks)docs/tasks-todo/task-8-extract-fileitem-component-fix-hover.md(1 hunks)src-tauri/src/commands/files.rs(17 hunks)src/store/__tests__/editorStore.integration.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: Do not use React Hook Form; prefer Direct Store Pattern to avoid infinite loops
Use react-hotkeys-hook for keyboard shortcuts (e.g., mod+s, mod+1)
Type store destructuring explicitly when using Zustand stores
Files:
src/store/__tests__/editorStore.integration.test.ts
src/store/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
In stores (no React hooks), dispatch DOM CustomEvents for cross-layer communication (Bridge Pattern)
Files:
src/store/__tests__/editorStore.integration.test.ts
src-tauri/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use modern Rust formatting: format("{variable}")
Files:
src-tauri/src/commands/files.rs
🧠 Learnings (2)
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/hooks/**/*.test.{ts,tsx} : Write integration tests for hooks and workflows under src/hooks/
Applied to files:
src/store/__tests__/editorStore.integration.test.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/components/**/*.test.{ts,tsx} : Write component tests for user interactions under src/components/
Applied to files:
src/store/__tests__/editorStore.integration.test.ts
🧬 Code graph analysis (2)
src/store/__tests__/editorStore.integration.test.ts (3)
src/store/editorStore.ts (2)
FileEntry(177-186)useEditorStore(221-474)src/store/projectStore.ts (1)
useProjectStore(46-428)src/test/mocks/toast.ts (1)
resetToastMocks(18-23)
src-tauri/src/commands/files.rs (1)
src-tauri/src/commands/mdx_components.rs (1)
validate_project_path(17-32)
🪛 LanguageTool
docs/tasks-todo/task-5-improve-rust-test-suite.md
[style] ~305-~305: To elevate your writing, try using a synonym here.
Context: ...❌ Permission error tests (OS-dependent, hard to mock reliably) - ❌ Fuzzing (nice but...
(HARD_TO)
docs/tasks-todo/task-8-extract-fileitem-component-fix-hover.md
[style] ~8-~8: To elevate your writing, try using a synonym here.
Context: ... inline JSX in .map() making the code hard to maintain ## Solution Extract file ...
(HARD_TO)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (10)
docs/tasks-todo/task-5-improve-rust-test-suite.md (2)
264-325: Implementation plan is pragmatic and realistic; approve overall approach.The step-by-step breakdown (5 steps, ~3.5 hours total) is realistic, and the focus on data integrity first is appropriate for a text editor. The effort distribution and success criteria are clear. Once the missing import preservation test is added (see earlier comment), this becomes a comprehensive testing plan.
112-123: Function signature verified—no changes needed.The function signature at
src-tauri/src/commands/files.rs:538-542confirms thatrebuild_markdown_with_frontmatter_and_importsacceptsfrontmatter: &IndexMap<String, Value>,imports: &str, andcontent: &str. The test calls match this signature exactly—all three arguments are in the correct order and types. No updates to the documentation are required.src/store/__tests__/editorStore.integration.test.ts (2)
1-11: LGTM! Clean mock setup.The query client mock correctly prevents hanging on
invalidateQueriescalls during tests. The mock structure matches the expected interface.
77-241: Excellent comprehensive coverage of auto-save behavior.The test suite thoroughly validates the auto-save implementation including:
- Max delay force save (10s threshold)
- Debounced save scheduling
- Dirty state transitions
- Timeout replacement logic
The use of fake timers makes these tests deterministic and fast.
docs/tasks-todo/task-8-extract-fileitem-component-fix-hover.md (1)
1-236: LGTM! Well-structured task documentation.This task documentation is comprehensive and well-organized, covering problem analysis, architectural considerations, implementation details, and testing criteria. The performance considerations and extraction rationale are particularly well thought out.
src-tauri/src/commands/files.rs (5)
2-4: LGTM! Appropriate imports for ordered frontmatter.The IndexMap and serde_norway imports support the migration to ordered frontmatter handling, which preserves key order as required by the PR objectives.
248-265: LGTM! Import preservation correctly implemented.The update_frontmatter function now correctly preserves MDX imports by passing parsed.imports to the rebuild function, addressing the bug mentioned in the PR objectives. Test coverage at lines 2115-2201 verifies that imports are preserved during frontmatter updates.
347-424: Excellent fix for import/Markdown boundary detection.The new is_markdown_block_start and is_numbered_list_start functions properly detect Markdown block starts, preventing content from being absorbed into import statements when semicolons are missing. The extract_imports_from_content loop now checks is_markdown_block_start before treating lines as import continuations, addressing the past review concern. Test coverage at lines 1420-1453 verifies this fix works correctly.
468-531: LGTM! Well-implemented normalization and ordering utilities.The normalize_dates function recursively converts ISO datetime strings to date-only format, ensuring clean frontmatter output. The build_ordered_frontmatter function orders keys with schema fields first, then remaining fields alphabetically, providing predictable and maintainable frontmatter structure. The recursive handling of nested objects and arrays is thorough.
1420-2453: Exceptional test coverage for edge cases and regressions.The test suite is comprehensive and production-ready:
- Regression test for import/Markdown boundary handling (lines 1420-1453)
- Regression test for MDX import preservation during frontmatter updates (lines 2115-2201)
- Unicode edge cases including emoji, RTL text, mixed scripts, and combining characters (lines 2207-2296)
- Malformed YAML handling (lines 2320-2373)
- Line ending variations including CRLF and mixed (lines 2379-2414)
- Empty and minimal input tests (lines 2420-2453)
This level of edge-case coverage significantly improves robustness for international users and real-world file variations.
1. Now calls updateFrontmatterField('title', 'New Title') - This is the actual operation that was causing imports to be lost, so we need to test it
2. Clears scheduled auto-save - Since we're using real timers in this test block, updateFrontmatterField would schedule an auto-save. We clear it and call saveFile manually for
deterministic timing
3. Changed initial isDirty: true to false - This makes the test more realistic, as the file starts clean and becomes dirty when we update the frontmatter
The test now properly validates that the bug fix works - when you update frontmatter on a file with imports, the imports are preserved during the save operation.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/layout/LeftSidebar.tsx (1)
187-214: Prevent no-op renames from calling the mutation
newName === file.namenever short-circuits becausefile.nameexcludes the extension whilenewNameincludes it, so we still invoke the rename mutation even when nothing changed. That triggers unnecessary backend work and can surface rename errors on a no-op. Compare against the full filename (including extension) before mutating.Apply this diff:
- if (!newName.trim() || newName === file.name) { + const originalName = file.extension + ? `${file.name}.${file.extension}` + : file.name + if (!newName.trim() || newName === originalName) { setRenamingFileId(null) return }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/tasks-done/task-4-fix-file-watcher-race-condition.md(1 hunks)docs/tasks-done/task-5-improve-rust-test-suite.md(1 hunks)docs/tasks-done/task-8-extract-fileitem-component-fix-hover.md(1 hunks)docs/tasks-todo/task-1-centralize-frontend-types.md(1 hunks)src/components/layout/FileItem.tsx(1 hunks)src/components/layout/LeftSidebar.tsx(7 hunks)src/store/__tests__/editorStore.integration.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/store/tests/editorStore.integration.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: Do not use React Hook Form; prefer Direct Store Pattern to avoid infinite loops
Use react-hotkeys-hook for keyboard shortcuts (e.g., mod+s, mod+1)
Type store destructuring explicitly when using Zustand stores
Files:
src/components/layout/LeftSidebar.tsxsrc/components/layout/FileItem.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/components/**/*.tsx: Components should access stores directly (Direct Store Pattern) instead of prop callbacks that cause dependency loops
Extract helper components when JSX is repeated 3+ times
Use AutoExpandingTextarea instead of CSS field-sizing: content in WebKit
Files:
src/components/layout/LeftSidebar.tsxsrc/components/layout/FileItem.tsx
src/components/**/[A-Z]*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Name React component files in PascalCase within src/components
Files:
src/components/layout/LeftSidebar.tsxsrc/components/layout/FileItem.tsx
src/components/[a-z0-9-]*/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Use kebab-case for component directory names under src/components
Files:
src/components/layout/LeftSidebar.tsxsrc/components/layout/FileItem.tsx
🧠 Learnings (3)
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to {src/lib,src/hooks}/**/*.{ts,tsx} : Invalidate TanStack Query caches on mutation success using the correct queryKey
Applied to files:
docs/tasks-done/task-4-fix-file-watcher-race-condition.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Track progress by updating docs/tasks-todo after major work
Applied to files:
docs/tasks-done/task-4-fix-file-watcher-race-condition.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/hooks/**/*.test.{ts,tsx} : Write integration tests for hooks and workflows under src/hooks/
Applied to files:
docs/tasks-done/task-4-fix-file-watcher-race-condition.md
🧬 Code graph analysis (2)
src/components/layout/LeftSidebar.tsx (3)
src/store/editorStore.ts (2)
useEditorStore(221-474)FileEntry(177-186)src/store/projectStore.ts (1)
useProjectStore(46-428)src/components/layout/FileItem.tsx (1)
FileItem(73-197)
src/components/layout/FileItem.tsx (1)
src/lib/utils.ts (1)
cn(4-6)
🪛 LanguageTool
docs/tasks-done/task-5-improve-rust-test-suite.md
[style] ~305-~305: To elevate your writing, try using a synonym here.
Context: ...❌ Permission error tests (OS-dependent, hard to mock reliably) - ❌ Fuzzing (nice but...
(HARD_TO)
docs/tasks-done/task-8-extract-fileitem-component-fix-hover.md
[style] ~8-~8: To elevate your writing, try using a synonym here.
Context: ... inline JSX in .map() making the code hard to maintain ## Solution Extract file ...
(HARD_TO)
docs/tasks-done/task-4-fix-file-watcher-race-condition.md
[style] ~12-~12: ‘on top of that’ might be wordy. Consider a shorter alternative.
Context: ... implements file watcher event handling on top of that architecture. ### Discovery The archi...
(EN_WORDINESS_PREMIUM_ON_TOP_OF_THAT)
[style] ~398-~398: Consider an alternative to strengthen your wording.
Context: ...lidation. Alternative (cleaner but more changes): Create hooks/useEditorSave.ts that ...
(CHANGES_ADJUSTMENTS)
[style] ~675-~675: To form a complete sentence, be sure to include a subject.
Context: ... v1.0.0 timeline: 4-5 hours total. Can be done in single session. --- ## Suc...
(MISSING_IT_THERE)
🪛 markdownlint-cli2 (0.18.1)
docs/tasks-done/task-4-fix-file-watcher-race-condition.md
98-98: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
815-815: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/tasks-todo/task-1-centralize-frontend-types.md
481-481: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Analyze (rust)
dannysmith
force-pushed
the
code-improvements-oct-2025
branch
from
d58a147 to
9898b33
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/frontmatter/fields/FrontmatterField.tsx (1)
29-71: Nested field values now come back as undefinedSwitching the selector to
state.frontmatter?.[name]only works for top-level keys. Many schema entries use dot-notation (e.g.seo.title,authors.0.name). Our store writes those viaupdateFrontmatterField, which stores nested structures usingsetNestedValue. With the new selector, any field whose name contains a dot no longer resolves its value, so the form renders blanks and overwriting these fields will wipe existing data. Please pull the value throughgetNestedValue(or an equivalent traversal) inside the selector so nested paths still resolve, and update the memo dependencies accordingly.
Suggested fix:-import { useEditorStore } from '../../../store/editorStore' +import { useEditorStore } from '../../../store/editorStore' +import { getNestedValue } from '../../../lib/object-utils' - const fieldValue = useEditorStore(state => state.frontmatter?.[name]) + const fieldValue = useEditorStore(state => + getNestedValue(state.frontmatter ?? {}, name) + )
♻️ Duplicate comments (1)
src/hooks/useCreateFile.ts (1)
220-220: Restore awaitingopenFile— regression from previous review.This appears to be a regression: the past review identified that
openFileis asynchronous, and dropping theawaitallowsisCreatingRefto flip back tofalsein the finally block beforeopenFilecompletes. This enables a fast double-click to schedule another create whilst the firstopenFileis in flight, and any rejection fromopenFilebypasses the try/catch entirely, resulting in an unhandled promise rejection.Apply this diff to restore the await and keep the concurrency guard effective:
- openFile(newFile) + await openFile(newFile)
🧹 Nitpick comments (6)
docs/tasks-todo/task-3-rust-refactorings.md (1)
1-640: Comprehensive and well-structured refactoring roadmap; ready for implementation.This document provides a clear, actionable plan for reducing Rust backend complexity across four focused refactoring items. Each item includes problem analysis, proposed solutions, step-by-step implementation guidance, testing strategy, and quality gates. The incremental approach and conservative risk assessment (leveraging existing test coverage) make this a solid foundation for this work.
Minor suggestions to enhance clarity:
Typographical consistency (lines 7, 24, 156, 286, 418, 634): Replace hyphens in time ranges with en-dashes (e.g.,
2-3 weeks→2–3 weeks;2-3 days→2–3 days). This aligns with formal typographical standards for ranges.Phrase repetition (line 206): The instruction list uses "Add" four times in succession. Consider restructuring for flow:
- 1. **Add helper functions** after `is_markdown_block_start` (around line 350): - - Add `is_import_or_export_line()` - - Add `should_continue_import()` - - Add `has_import_terminator()` - - Add doc comments explaining each function's purpose + 1. **Add helper functions** after `is_markdown_block_start` (around line 350): + - `is_import_or_export_line()` + - `should_continue_import()` + - `has_import_terminator()` + - Doc comments explaining each function's purpose
Potential clarification (line 338): The phrase "Combine into complete path" could be clearer. Consider: "Combine results into complete field path".
Document status tracking: Since the learnings note suggests tracking progress in
docs/tasks-todoafter major work, consider adding a Progress section or comment at the document's top that can be updated as items complete (e.g.,- [ ] Item 1,- [x] Item 1, etc.). This will help maintain visibility when cross-referencing fromCLAUDE.md.docs/reviews/TESTING-REVIEW.md (2)
1-37: Capitalise "Markdown" as a proper noun throughout the document.The document currently uses "markdown" in several places (e.g., line 13). As "Markdown" is the proper name of the formatting language, it should be capitalised consistently. This applies at line 13 ("markdown formatting"), line 161 ("Invalid markdown syntax"), and any other instances.
- - Excellent unit tests for pure business logic (markdown formatting, schema parsing, field utilities) + - Excellent unit tests for pure business logic (Markdown formatting, schema parsing, field utilities)And at line 161:
- - Invalid markdown syntax + - Invalid Markdown syntax
393-408: Specify a language identifier for the fenced code block.The coverage statistics code block (line 395) lacks a language identifier, which violates markdown linting rule MD040. Specify a language such as
text,plaintext, orcoverageto fix this linting issue.-``` +```text src/components/frontmatter/fields : ~75% (good component testing)This enables proper syntax highlighting in documentation renderers and complies with markdown standards.
src/hooks/settings/useEffectiveSettings.ts (1)
15-16: Switch to a selector when reading from the project storeCalling
useProjectStore()without a selector subscribes to the entire store, so every project-store update will re-render any consumer ofuseEffectiveSettings. Please select onlycurrentProjectSettingsto avoid unnecessary renders.-export const useEffectiveSettings = (collectionName?: string) => { - const { currentProjectSettings } = useProjectStore() - return getEffectiveSettings(currentProjectSettings, collectionName) -} +export const useEffectiveSettings = (collectionName?: string) => { + const currentProjectSettings = useProjectStore( + state => state.currentProjectSettings + ) + return getEffectiveSettings(currentProjectSettings, collectionName) +}As per coding guidelines
src/lib/files/sorting.ts (1)
7-12: Extract the sharedFieldMappingstype.
We now declare identicalFieldMappingsshapes here and in src/lib/files/filtering.ts (lines 7-12). Keeping two copies in sync is brittle; please extract a single export (for example in src/lib/files/types.ts or src/types/files.ts) and import it in both modules.src/components/layout/LeftSidebar.tsx (1)
32-37: Subscribe to individual store slicesCalling
useProjectStore()without a selector subscribes LeftSidebar to every state change, which undoes the render optimisations you added elsewhere. Please pull each field with its own selector (or a shallow selector group) so the sidebar only re-renders when the relevant slice mutates. As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (47)
CLAUDE.md(3 hunks)docs/developer/architecture-guide.md(2 hunks)docs/reviews/ARCHITECTURE-REVIEW.md(1 hunks)docs/reviews/TESTING-REVIEW.md(1 hunks)docs/reviews/refactoring-opportunities.md(1 hunks)docs/tasks-done/task-2025-11-01-quick-wins-architecture-and-utilities.md(1 hunks)docs/tasks-todo/task-2-high-value-testing.md(1 hunks)docs/tasks-todo/task-3-rust-refactorings.md(1 hunks)docs/tasks-todo/task-4-advanced-frontend-refactorings.md(1 hunks)src/components/component-builder/ComponentBuilderDialog.tsx(1 hunks)src/components/editor/Editor.tsx(2 hunks)src/components/frontmatter/fields/ArrayField.tsx(2 hunks)src/components/frontmatter/fields/BooleanField.tsx(2 hunks)src/components/frontmatter/fields/DateField.tsx(3 hunks)src/components/frontmatter/fields/EnumField.tsx(4 hunks)src/components/frontmatter/fields/FrontmatterField.tsx(4 hunks)src/components/frontmatter/fields/ImageField.tsx(2 hunks)src/components/frontmatter/fields/NumberField.tsx(2 hunks)src/components/frontmatter/fields/ReferenceField.tsx(4 hunks)src/components/frontmatter/fields/StringField.tsx(2 hunks)src/components/frontmatter/fields/TextareaField.tsx(2 hunks)src/components/frontmatter/fields/YamlField.tsx(2 hunks)src/components/layout/FileItem.tsx(1 hunks)src/components/layout/LeftSidebar.tsx(11 hunks)src/hooks/commands/index.ts(1 hunks)src/hooks/commands/useCommandContext.ts(1 hunks)src/hooks/mutations/useDeleteFileMutation.ts(2 hunks)src/hooks/mutations/useRenameFileMutation.ts(2 hunks)src/hooks/mutations/useSaveFileMutation.ts(2 hunks)src/hooks/queries/useFileContentQuery.ts(3 hunks)src/hooks/settings/index.ts(1 hunks)src/hooks/settings/useEffectiveSettings.ts(1 hunks)src/hooks/useCommandPalette.ts(1 hunks)src/hooks/useCreateFile.ts(5 hunks)src/hooks/useEditorFileContent.ts(1 hunks)src/hooks/useFileChangeHandler.ts(1 hunks)src/lib/commands/index.ts(0 hunks)src/lib/files/filtering.test.ts(1 hunks)src/lib/files/filtering.ts(1 hunks)src/lib/files/index.ts(1 hunks)src/lib/files/sorting.test.ts(1 hunks)src/lib/files/sorting.ts(1 hunks)src/lib/index.ts(1 hunks)src/lib/object-utils.test.ts(1 hunks)src/lib/object-utils.ts(1 hunks)src/lib/project-registry/effective-settings.ts(0 hunks)src/store/editorStore.ts(7 hunks)
💤 Files with no reviewable changes (2)
- src/lib/commands/index.ts
- src/lib/project-registry/effective-settings.ts
✅ Files skipped from review due to trivial changes (2)
- src/components/component-builder/ComponentBuilderDialog.tsx
- src/hooks/useCommandPalette.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- src/components/frontmatter/fields/EnumField.tsx
- CLAUDE.md
- src/components/frontmatter/fields/TextareaField.tsx
- src/components/frontmatter/fields/BooleanField.tsx
- src/components/frontmatter/fields/YamlField.tsx
- src/components/frontmatter/fields/ImageField.tsx
- src/components/layout/FileItem.tsx
- src/components/frontmatter/fields/ArrayField.tsx
- src/components/frontmatter/fields/ReferenceField.tsx
- src/hooks/useFileChangeHandler.ts
- src/hooks/useEditorFileContent.ts
- src/components/editor/Editor.tsx
- src/components/frontmatter/fields/NumberField.tsx
🧰 Additional context used
📓 Path-based instructions (12)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: Do not use React Hook Form; prefer Direct Store Pattern to avoid infinite loops
Use react-hotkeys-hook for keyboard shortcuts (e.g., mod+s, mod+1)
Type store destructuring explicitly when using Zustand stores
Files:
src/lib/files/sorting.tssrc/hooks/commands/index.tssrc/lib/index.tssrc/hooks/settings/index.tssrc/components/frontmatter/fields/FrontmatterField.tsxsrc/components/frontmatter/fields/StringField.tsxsrc/lib/object-utils.test.tssrc/hooks/mutations/useRenameFileMutation.tssrc/lib/files/filtering.tssrc/hooks/mutations/useDeleteFileMutation.tssrc/lib/files/filtering.test.tssrc/components/layout/LeftSidebar.tsxsrc/hooks/useCreateFile.tssrc/lib/files/sorting.test.tssrc/hooks/commands/useCommandContext.tssrc/hooks/settings/useEffectiveSettings.tssrc/hooks/mutations/useSaveFileMutation.tssrc/components/frontmatter/fields/DateField.tsxsrc/lib/object-utils.tssrc/hooks/queries/useFileContentQuery.tssrc/lib/files/index.tssrc/store/editorStore.ts
{src/lib,src/hooks}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Invalidate TanStack Query caches on mutation success using the correct queryKey
Files:
src/lib/files/sorting.tssrc/hooks/commands/index.tssrc/lib/index.tssrc/hooks/settings/index.tssrc/lib/object-utils.test.tssrc/hooks/mutations/useRenameFileMutation.tssrc/lib/files/filtering.tssrc/hooks/mutations/useDeleteFileMutation.tssrc/lib/files/filtering.test.tssrc/hooks/useCreateFile.tssrc/lib/files/sorting.test.tssrc/hooks/commands/useCommandContext.tssrc/hooks/settings/useEffectiveSettings.tssrc/hooks/mutations/useSaveFileMutation.tssrc/lib/object-utils.tssrc/hooks/queries/useFileContentQuery.tssrc/lib/files/index.ts
src/lib/**
📄 CodeRabbit inference engine (CLAUDE.md)
Extract complex, testable business logic (50+ lines or 2+ components) into src/lib/ modules
Files:
src/lib/files/sorting.tssrc/lib/index.tssrc/lib/object-utils.test.tssrc/lib/files/filtering.tssrc/lib/files/filtering.test.tssrc/lib/files/sorting.test.tssrc/lib/object-utils.tssrc/lib/files/index.ts
src/hooks/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place reusable React hooks and side-effect logic in src/hooks/
Files:
src/hooks/commands/index.tssrc/hooks/settings/index.tssrc/hooks/mutations/useRenameFileMutation.tssrc/hooks/mutations/useDeleteFileMutation.tssrc/hooks/useCreateFile.tssrc/hooks/commands/useCommandContext.tssrc/hooks/settings/useEffectiveSettings.tssrc/hooks/mutations/useSaveFileMutation.tssrc/hooks/queries/useFileContentQuery.ts
src/**/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use index.ts barrel exports for component/modules directories
Files:
src/hooks/commands/index.tssrc/lib/index.tssrc/hooks/settings/index.tssrc/lib/files/index.ts
src/components/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/components/**/*.tsx: Components should access stores directly (Direct Store Pattern) instead of prop callbacks that cause dependency loops
Extract helper components when JSX is repeated 3+ times
Use AutoExpandingTextarea instead of CSS field-sizing: content in WebKit
Files:
src/components/frontmatter/fields/FrontmatterField.tsxsrc/components/frontmatter/fields/StringField.tsxsrc/components/layout/LeftSidebar.tsxsrc/components/frontmatter/fields/DateField.tsx
src/components/**/[A-Z]*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Name React component files in PascalCase within src/components
Files:
src/components/frontmatter/fields/FrontmatterField.tsxsrc/components/frontmatter/fields/StringField.tsxsrc/components/layout/LeftSidebar.tsxsrc/components/frontmatter/fields/DateField.tsx
src/components/[a-z0-9-]*/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Use kebab-case for component directory names under src/components
Files:
src/components/frontmatter/fields/FrontmatterField.tsxsrc/components/frontmatter/fields/StringField.tsxsrc/components/layout/LeftSidebar.tsxsrc/components/frontmatter/fields/DateField.tsx
src/lib/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Write unit tests for modules in src/lib/
Files:
src/lib/object-utils.test.tssrc/lib/files/filtering.test.tssrc/lib/files/sorting.test.ts
docs/developer/architecture-guide.md
📄 CodeRabbit inference engine (CLAUDE.md)
Review docs/developer/architecture-guide.md before development
Files:
docs/developer/architecture-guide.md
docs/developer/**
📄 CodeRabbit inference engine (CLAUDE.md)
Update docs/developer/ guides when introducing new patterns
Files:
docs/developer/architecture-guide.md
src/store/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
In stores (no React hooks), dispatch DOM CustomEvents for cross-layer communication (Bridge Pattern)
Files:
src/store/editorStore.ts
🧠 Learnings (25)
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Track progress by updating docs/tasks-todo after major work
Applied to files:
docs/tasks-todo/task-3-rust-refactorings.mddocs/tasks-todo/task-4-advanced-frontend-refactorings.mddocs/tasks-todo/task-2-high-value-testing.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/lib/commands/command-context.ts : Add or update command context functions in src/lib/commands/command-context.ts
Applied to files:
src/hooks/commands/index.tssrc/hooks/commands/useCommandContext.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/lib/commands/**/*.ts : Use the Command Pattern via globalCommandRegistry for editor operations
Applied to files:
src/hooks/commands/index.tssrc/hooks/commands/useCommandContext.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/lib/commands/app-commands.ts : Define app-level commands in src/lib/commands/app-commands.ts
Applied to files:
src/hooks/commands/index.tssrc/hooks/commands/useCommandContext.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/hooks/useCommandPalette.ts : Update command group order in src/hooks/useCommandPalette.ts
Applied to files:
src/hooks/commands/index.tsdocs/tasks-done/task-2025-11-01-quick-wins-architecture-and-utilities.mdsrc/hooks/commands/useCommandContext.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/lib/commands/types.ts : Add new command groups by updating CommandGroup type in src/lib/commands/types.ts
Applied to files:
src/hooks/commands/index.tssrc/hooks/commands/useCommandContext.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/**/index.ts : Use index.ts barrel exports for component/modules directories
Applied to files:
src/hooks/commands/index.tssrc/lib/index.tssrc/lib/files/index.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/hooks/**/*.ts : Place reusable React hooks and side-effect logic in src/hooks/
Applied to files:
src/hooks/settings/index.tsdocs/tasks-done/task-2025-11-01-quick-wins-architecture-and-utilities.mddocs/developer/architecture-guide.mdsrc/hooks/settings/useEffectiveSettings.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/components/**/*.tsx : Use AutoExpandingTextarea instead of CSS field-sizing: content in WebKit
Applied to files:
src/components/frontmatter/fields/FrontmatterField.tsx
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/**/*.{ts,tsx} : Do not use React Hook Form; prefer Direct Store Pattern to avoid infinite loops
Applied to files:
src/components/frontmatter/fields/FrontmatterField.tsxsrc/components/frontmatter/fields/StringField.tsxsrc/components/layout/LeftSidebar.tsxdocs/developer/architecture-guide.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/**/*.{ts,tsx} : Type store destructuring explicitly when using Zustand stores
Applied to files:
src/components/frontmatter/fields/StringField.tsxdocs/developer/architecture-guide.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/lib/**/*.test.{ts,tsx} : Write unit tests for modules in src/lib/
Applied to files:
src/lib/object-utils.test.tssrc/lib/files/filtering.test.tssrc/lib/files/sorting.test.tsdocs/developer/architecture-guide.mddocs/tasks-todo/task-2-high-value-testing.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/hooks/**/*.test.{ts,tsx} : Write integration tests for hooks and workflows under src/hooks/
Applied to files:
src/lib/files/filtering.test.tsdocs/tasks-todo/task-2-high-value-testing.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/components/**/*.test.{ts,tsx} : Write component tests for user interactions under src/components/
Applied to files:
src/lib/files/filtering.test.tsdocs/tasks-todo/task-2-high-value-testing.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/**/*.{ts,tsx} : Use react-hotkeys-hook for keyboard shortcuts (e.g., mod+s, mod+1)
Applied to files:
docs/tasks-done/task-2025-11-01-quick-wins-architecture-and-utilities.mddocs/developer/architecture-guide.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to docs/developer/architecture-guide.md : Review docs/developer/architecture-guide.md before development
Applied to files:
docs/developer/architecture-guide.mddocs/reviews/ARCHITECTURE-REVIEW.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to docs/developer/** : Update docs/developer/ guides when introducing new patterns
Applied to files:
docs/developer/architecture-guide.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/components/**/*.tsx : Components should access stores directly (Direct Store Pattern) instead of prop callbacks that cause dependency loops
Applied to files:
docs/developer/architecture-guide.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/lib/** : Extract complex, testable business logic (50+ lines or 2+ components) into src/lib/ modules
Applied to files:
docs/developer/architecture-guide.mddocs/reviews/ARCHITECTURE-REVIEW.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/store/**/*.ts : In stores (no React hooks), dispatch DOM CustomEvents for cross-layer communication (Bridge Pattern)
Applied to files:
docs/developer/architecture-guide.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to package.json : Use React 19.x, shadcn/ui v4.x, Tailwind v4.x, Zustand v5.x, TanStack Query v5, Vitest v3 in package.json
Applied to files:
docs/developer/architecture-guide.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/components/**/*.tsx : Extract helper components when JSX is repeated 3+ times
Applied to files:
docs/developer/architecture-guide.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to {src/lib,src/hooks}/**/*.{ts,tsx} : Invalidate TanStack Query caches on mutation success using the correct queryKey
Applied to files:
src/hooks/mutations/useSaveFileMutation.tssrc/hooks/queries/useFileContentQuery.tssrc/store/editorStore.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/lib/query-keys.ts : Use a centralized TanStack Query keys factory in src/lib/query-keys.ts
Applied to files:
src/hooks/queries/useFileContentQuery.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Use memoization, debouncing (2s auto-save), and getState() to prevent render cascades
Applied to files:
src/store/editorStore.ts
🧬 Code graph analysis (14)
src/lib/files/sorting.ts (2)
src/lib/files/index.ts (2)
getPublishedDate(17-17)sortFilesByPublishedDate(17-17)src/types/domain.ts (1)
FileEntry(14-38)
src/components/frontmatter/fields/FrontmatterField.tsx (1)
src/store/editorStore.ts (1)
useEditorStore(40-301)
src/components/frontmatter/fields/StringField.tsx (2)
src/store/editorStore.ts (1)
useEditorStore(40-301)src/lib/object-utils.ts (1)
getNestedValue(73-91)
src/lib/object-utils.test.ts (1)
src/lib/object-utils.ts (3)
setNestedValue(10-67)getNestedValue(73-91)deleteNestedValue(97-169)
src/lib/files/filtering.ts (2)
src/lib/files/index.ts (1)
filterFilesByDraft(16-16)src/types/domain.ts (1)
FileEntry(14-38)
src/lib/files/filtering.test.ts (2)
src/types/domain.ts (1)
FileEntry(14-38)src/lib/files/filtering.ts (1)
filterFilesByDraft(23-40)
src/components/layout/LeftSidebar.tsx (7)
src/store/editorStore.ts (1)
useEditorStore(40-301)src/store/projectStore.ts (1)
useProjectStore(46-428)src/types/domain.ts (1)
FileEntry(14-38)src/lib/files/filtering.ts (1)
filterFilesByDraft(23-40)src/lib/files/sorting.ts (1)
sortFilesByPublishedDate(50-74)src/lib/projects/actions.ts (1)
openProjectViaDialog(5-18)src/components/layout/FileItem.tsx (1)
FileItem(54-178)
src/hooks/useCreateFile.ts (3)
src/lib/dates.ts (1)
todayIsoDate(5-7)src/store/projectStore.ts (1)
useProjectStore(46-428)src/lib/project-registry/default-file-type.ts (1)
getDefaultFileType(28-55)
src/lib/files/sorting.test.ts (2)
src/types/domain.ts (1)
FileEntry(14-38)src/lib/files/sorting.ts (2)
getPublishedDate(22-40)sortFilesByPublishedDate(50-74)
src/hooks/settings/useEffectiveSettings.ts (2)
src/store/projectStore.ts (1)
useProjectStore(46-428)src/lib/project-registry/effective-settings.ts (1)
getEffectiveSettings(16-84)
src/components/frontmatter/fields/DateField.tsx (3)
src/store/editorStore.ts (1)
useEditorStore(40-301)src/lib/object-utils.ts (1)
getNestedValue(73-91)src/lib/dates.ts (1)
formatIsoDate(1-3)
src/lib/object-utils.ts (1)
src/lib/index.ts (3)
setNestedValue(2-2)getNestedValue(3-3)deleteNestedValue(4-4)
src/hooks/queries/useFileContentQuery.ts (1)
src/lib/query-keys.ts (1)
queryKeys(3-40)
src/store/editorStore.ts (2)
src/types/domain.ts (1)
FileEntry(14-38)src/lib/query-keys.ts (1)
queryKeys(3-40)
🪛 LanguageTool
docs/tasks-todo/task-3-rust-refactorings.md
[typographical] ~7-~7: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...schema merger modules. Total Time: 2-3 weeks (can be done incrementally) **Im...
(HYPHEN_TO_EN)
[typographical] ~24-~24: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...s (146-206, 294-335, 338-368) Time: 2-3 days Risk: LOW ### Current Issues ...
(HYPHEN_TO_EN)
[typographical] ~156-~156: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...ports_from_content` function) Time: 2-3 days Risk: LOW (comprehensive tests...
(HYPHEN_TO_EN)
[grammar] ~206-~206: This phrase is duplicated. You should probably use “Add Add” only once.
Context: ...wn_block_start(around line 350): - Addis_import_or_export_line() - Addshould_continue_import() - Addhas_import_terminator()` - Add doc comments explaining each function's...
(PHRASE_REPETITION)
[typographical] ~286-~286: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...resolve_field_path` function) Time: 2-3 days Risk: LOW-MEDIUM (good test co...
(HYPHEN_TO_EN)
[uncategorized] ~338-~338: Possible missing preposition found.
Context: ...uild_parent_path()` to get parents - Combine into complete path - Keep existing e...
(AI_HYDRA_LEO_MISSING_TO)
[typographical] ~418-~418: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...iple long functions (260-616) Time: 4-5 days Risk: MEDIUM (core schema proc...
(HYPHEN_TO_EN)
[typographical] ~634-~634: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...t 3 --- ## Notes - Total effort: 2-3 weeks (can be done incrementally) - **E...
(HYPHEN_TO_EN)
docs/tasks-todo/task-4-advanced-frontend-refactorings.md
[typographical] ~28-~28: If specifying a range, consider using an en dash instead of a hyphen.
Context: ... handles all POS highlighting Time: 5-7 days Risk: MEDIUM (user-facing feat...
(HYPHEN_TO_EN)
[typographical] ~330-~330: If specifying a range, consider using an en dash instead of a hyphen.
Context: ....md --- ## Notes - Total effort: 5-7 days (~1 week) - **User-facing feature:...
(HYPHEN_TO_EN)
docs/tasks-done/task-2025-11-01-quick-wins-architecture-and-utilities.md
[typographical] ~7-~7: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...ings and testing work. Total Time: 10-15 hours (~1-2 days) Impact: - Clear ...
(HYPHEN_TO_EN)
[typographical] ~40-~40: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...- ## Phase 1: Architecture Boundaries (4-6 hours) Fix directory convention violat...
(HYPHEN_TO_EN)
[typographical] ~47-~47: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...ommands/useCommandContext.ts` Time: 2-3 hours Risk: LOW #### Current Issue...
(HYPHEN_TO_EN)
[typographical] ~91-~91: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...lit into hook + pure function Time: 2-3 hours Risk: LOW-MEDIUM (need to spl...
(HYPHEN_TO_EN)
[uncategorized] ~108-~108: Possible missing comma found.
Context: ...ction (should stay in lib/) } ``` File exports both hook AND pure function. Need to sp...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~155-~155: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...** --- ## Phase 2: Extract Utilities (4-6 hours) Extract pure functions from com...
(HYPHEN_TO_EN)
[typographical] ~162-~162: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...→src/lib/object-utils.ts` Time: 2-3 hours Risk: LOW Reference: See...
(HYPHEN_TO_EN)
[typographical] ~208-~208: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...debar.tsx→src/lib/files/` Time: 3-4 hours Risk: 🔴 **HIGHEST RISK - EXT...
(HYPHEN_TO_EN)
[typographical] ~509-~509: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...``` --- ## Notes - Total effort: 10-15 hours (~1-2 days) - Risk level: Var...
(HYPHEN_TO_EN)
[uncategorized] ~511-~511: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...OW to HIGHEST) - Items 1.1, 1.2, 2.1: LOW risk - Item 2.2: 🔴 HIGHEST RISK - caused ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~528-~528: Consider an alternative to strengthen your wording.
Context: ...m 2.2 STOP. Don't try to fix it with more changes. 1. Revert Item 2.2 changes: `git ch...
(CHANGES_ADJUSTMENTS)
docs/reviews/ARCHITECTURE-REVIEW.md
[uncategorized] ~13-~13: A comma may be missing after the conjunctive/linking adverb ‘However’.
Context: ...architecture is generally respected. However, there are **three architectural fric...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~15-~15: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ore Circular Dependency** (10 files) 🔴 HIGH PRIORITY 2. *Hooks Exported from Lib Directory...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~16-~16: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...orted from Lib Directory** (3 hooks) 🔴 HIGH PRIORITY 3. **Large, Multi-Responsibility Compon...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[typographical] ~17-~17: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...i-Responsibility Components** (4 files, 400-700 lines) 🟡 MEDIUM PRIORITY **Overall Gr...
(HYPHEN_TO_EN)
[uncategorized] ~19-~19: Possible missing comma found.
Context: ...IUM PRIORITY Overall Grade: B+ (Very Good with Specific Improvement Areas) ---...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~344-~344: Possible typo detected: Did you mean to write ‘from’ instead of ‘form’?
Context: ...der/ComponentBuilderDialog.tsx` | 398 | Form + preview + file management | ### Why ...
(TYPO_FORM_FROM)
[misspelling] ~364-~364: This word is normally spelled as one.
Context: ...er to optimize rendering (can't memoize sub-sections easily) - All logic re-runs on any stat...
(EN_COMPOUNDS_SUB_SECTIONS)
[uncategorized] ~373-~373: Possible missing article found.
Context: ...e customizing it heavily and it becomes maintenance burden. #### 3.2 `CollectionSettingsPa...
(AI_HYDRA_LEO_MISSING_A)
[typographical] ~585-~585: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...ts (header, nav, list) - Effort: 4-6 hours 2. *CollectionSettingsPane.tsx...
(HYPHEN_TO_EN)
[style] ~589-~589: Consider using a different adjective in this context to strengthen your wording.
Context: ...mplex but less frequently modified - Good candidate for extraction (clear setting...
(GOOD_ALTERNATIVE)
[typographical] ~590-~590: If specifying a range, consider using an en dash instead of a hyphen.
Context: ... (clear setting types) - Effort: 3-4 hours 3. *ComponentBuilderDialog.tsx...
(HYPHEN_TO_EN)
[typographical] ~596-~596: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...ature expansion needed - Effort: 2-3 hours 4. ui/sidebar.tsx (Skip) ...
(HYPHEN_TO_EN)
[grammar] ~606-~606: A determiner may be missing.
Context: ...y** for related components 2. Extract dumbest component first (pure presentation) 3...
(THE_SUPERLATIVE)
[misspelling] ~628-~628: This word is normally spelled as one.
Context: ...- [ ] Complex components have extracted sub-components with tests - [ ] File/directory naming ...
(EN_COMPOUNDS_SUB_COMPONENTS)
[duplication] ~827-~827: Possible typo: you repeated a word.
Context: ...nts** → Can import from hooks, lib, and store 4. Store → Can import from lib (utilities), no...
(ENGLISH_WORD_REPEAT_RULE)
[style] ~910-~910: To elevate your writing, try using a synonym here.
Context: ...endency | High - violates architecture, hard to test | Medium (8 hours) | 10 files |...
(HARD_TO)
[uncategorized] ~913-~913: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ns | Low (2 hours) | 3 files | Total High Priority Effort: ~10 hours ### 🟡 Medium Prio...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~919-~919: To elevate your writing, try using a synonym here.
Context: ...--------| | Large components | Medium - hard to maintain, test, understand | Medium-...
(HARD_TO)
[uncategorized] ~922-~922: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...n | Low (1 hour) | ~20 files | Total Medium Priority Effort: ~11 hours ### 🟢 Low Priorit...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~932-~932: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ues | Low (4 hours) | 1 file | Total Low Priority Effort: ~10 hours --- ## 9. Conclus...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~957-~957: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... hours (3 days) to address all high and medium priority items. Expected Outcome: - A-grade...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
docs/reviews/TESTING-REVIEW.md
[uncategorized] ~13-~13: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...ent unit tests for pure business logic (markdown formatting, schema parsing, field utili...
(MARKDOWN_NNP)
[uncategorized] ~161-~161: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ... errors - Disk full scenarios - Invalid markdown syntax - Corrupted frontmatter **Perfo...
(MARKDOWN_NNP)
[style] ~308-~308: To elevate your writing, try using a synonym here.
Context: ...oring for Testability:** - Some code is hard to test due to tight coupling (e.g., co...
(HARD_TO)
[duplication] ~361-~361: Possible typo: you repeated a word.
Context: ...tmatter/fields/FieldWrapper.test.tsx- ⚠️components/frontmatter/fields/FrontmatterField.test.tsx- ⚠️components/frontmatter/FrontmatterPane...
(ENGLISH_WORD_REPEAT_RULE)
docs/reviews/refactoring-opportunities.md
[style] ~8-~8: “low-hanging fruit” can be a clichéd phrase in professional communication. Consider an alternative expression to make your writing more engaging and original.
Context: ...aller, more focused functions—primarily low-hanging fruit that offers clear value without major r...
(LOW_HANGING_FRUIT)
[uncategorized] ~25-~25: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... coverage to validate changes. --- ## High Priority Refactorings ### 1. Extract Frontmatte...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~155-~155: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...n specific POS highlights fail --- ## Medium Priority Refactorings ### 4. Extract Schema Mer...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~384-~384: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... duplication (~30 lines saved) --- ## Low Priority Refactorings ### 9. Consider Extractin...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~429-~429: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...ause the current structure is reasonable and the duplication is relatively minor. On...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~460-~460: Possible missing preposition found.
Context: ...quires careful testing and validation - Performance-sensitive areas Order: 1. #4 (sche...
(AI_HYDRA_LEO_MISSING_OF)
[typographical] ~503-~503: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...omplexity - Average component size: 200-300 lines (acceptable) - **Hook complexity:...
(HYPHEN_TO_EN)
[typographical] ~511-~511: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...t Backend:** - Average function length: 30-40 lines (↓30%) - Cyclomatic complexity: 5...
(HYPHEN_TO_EN)
[typographical] ~517-~517: If specifying a range, consider using an en dash instead of a hyphen.
Context: ... Frontend:** - Average function length: 20-30 lines (↓25%) - Component size: <250 lin...
(HYPHEN_TO_EN)
[typographical] ~552-~552: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...rns emerge Total Estimated Effort: 4-6 weeks for all items (can be spread over...
(HYPHEN_TO_EN)
docs/tasks-todo/task-2-high-value-testing.md
[typographical] ~11-~11: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...(remove orphaned test) Total Time: 1-2 weeks Why This Matters: - Schema p...
(HYPHEN_TO_EN)
[typographical] ~26-~26: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...hema.ts(currently untested) **Time:** 2-3 days ### Current State Thedeseriali...
(HYPHEN_TO_EN)
[typographical] ~122-~122: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...eeds complete data flow tests Time: 3-4 days ### Test Suites to Add #### A. F...
(HYPHEN_TO_EN)
[typographical] ~204-~204: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...om Task 1 Priority: HIGH Time: 1-2 days ### Utilities to Test Now that T...
(HYPHEN_TO_EN)
[typographical] ~245-~245: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...ultiple test files >300 lines Time: 2-3 days Note: This was Item 8 in the ...
(HYPHEN_TO_EN)
[grammar] ~473-~473: It appears that a pronoun is missing.
Context: ...n test:coverage ``` --- ## Notes - Depends on Task 1: Utilities extracted in Tas...
(SENT_START_DEPENDS)
[typographical] ~474-~474: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...1 need testing here - Total effort: 1-2 weeks - Critical for safety: Schema...
(HYPHEN_TO_EN)
🪛 markdownlint-cli2 (0.18.1)
docs/reviews/TESTING-REVIEW.md
10-10: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
223-223: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
229-229: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
235-235: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
241-241: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
250-250: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
257-257: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
264-264: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
271-271: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
279-279: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
286-286: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
337-337: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
349-349: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
357-357: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
367-367: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
376-376: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
383-383: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
390-390: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
395-395: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (25)
docs/developer/architecture-guide.md (2)
22-52: Excellent clarification of directory boundaries and hook placement rules.This section provides much-needed specificity around a critical pattern. The table format is clear, and the good/bad examples effectively illustrate the rule that React hooks must live in
hooks/, notlib/. The documented getState() exception is particularly helpful for avoiding confusion.
230-268: All documented utility modules and exports verified—no issues found.Verification confirms that all four utility modules exist at their documented paths and export the functions and constants shown:
src/lib/dates.tsexportsformatIsoDateandtodayIsoDate✓src/lib/ide.tsexportsopenInIde✓src/lib/projects/actions.tsexportsopenProjectViaDialog(async function) ✓src/components/frontmatter/fields/constants.tsexportsNONE_SENTINEL✓The documentation is accurate and ready.
docs/reviews/TESTING-REVIEW.md (4)
38-175: Excellent findings and coverage analysis.This section demonstrates thorough investigation with specific, actionable examples. The categorisation of critical gaps, recommended additions, and nice-to-have items is well-reasoned and will guide the testing roadmap effectively.
307-332: Well-articulated guidance on refactoring for testability and anti-patterns.The refactoring example (lines 310–325) and anti-patterns guidance are clear, practical, and will help the team avoid common testing mistakes. This section elevates the document's utility beyond assessment into actionable guidance.
333-409: Excellent test inventory by category and coverage statistics.The appendix provides a comprehensive categorised inventory and coverage breakdown by directory. This reference material will be invaluable for prioritising testing efforts and tracking progress against the roadmap. The structure (checkmarks, warning indicators, cross-marks) effectively communicates test health at a glance.
410-412: Document structure and content quality is strong.This testing review demonstrates thorough investigation, clear prioritisation, and actionable recommendations. The assessment is well-grounded in specific examples and will provide a solid foundation for the testing roadmap. Beyond the markdown formatting issues flagged separately, the document is ready for use.
docs/tasks-todo/task-4-advanced-frontend-refactorings.md (2)
1-20: Clarify: Is this task planning document part of the current PR or a forward-looking roadmap?This file documents Task 4 (Advanced Frontend Refactorings) with a status of "Ready for implementation (after Tasks 1-3 complete)" (line 351). However, the PR #47 objectives summary indicates Tasks 1–3 are already complete. The PR appears to focus on other improvements (YAML parser, file watcher, auto-save, frontend type centralisation), not the POS highlighting decomposition described in this document.
This could mean either:
- Task 4 is a separate future task and this doc is a forward-looking roadmap (appropriate for merge), or
- Task 4 is part of this PR but the doc's status line is outdated.
Clarify the intent so it's obvious to future reviewers whether this is a "next steps" document or part of the current PR's scope. If it's future work, consider whether it should be merged now or deferred to a subsequent PR.
Also applies to: 351-352
40-207: Solid refactoring roadmap with well-thought-out extraction strategy.The proposed decomposition pattern (shared validation builder + per-POS processors) follows good separation of concerns. The implementation steps are detailed and practical:
- Validation logic extraction reduces duplication
- Per-POS processors enable independent optimisation
- Testing strategy includes performance benchmarking (reasonable 10% tolerance for refactoring)
- Alternative fallback approach shows realistic risk awareness
Aligns well with learnings about extracting complex business logic into testable modules.
docs/reviews/ARCHITECTURE-REVIEW.md (1)
1-20: Comprehensive architecture review with clear prioritisation and actionable recommendations.The executive summary is well-calibrated: acknowledging strong foundations (B+ grade) while identifying specific friction points (lib→store circular dependency, hooks in lib/, large components). The phased approach (dependency fixes → component extraction → documentation) is sensible and prioritises correctness over polish.
The positive patterns section (4.1–4.5) is particularly valuable, as it documents what's working well alongside what needs improvement. This helps future developers distinguish "don't follow this pattern" from "follow this pattern".
src/hooks/commands/index.ts (1)
1-1: LGTM! Follows barrel export pattern.The barrel export correctly re-exports
useCommandContext, making it available through the package index. This aligns with the coding guideline for using index.ts barrel exports.src/hooks/commands/useCommandContext.ts (1)
4-5: Import path updates look correct.The import paths have been updated as part of the code reorganization effort. The new paths align with the PR's objective to centralize frontend types and improve code structure:
useCollectionsQuerynow imported from the queries subdirectoryCommandContexttype imported from the commands library types moduleThese non-functional changes improve code organization without affecting runtime behaviour.
src/hooks/useCreateFile.ts (4)
1-12: LGTM: Import changes align with centralized types.The FileEntry type import from
@/typesand the new utility imports (todayIsoDate,getDefaultFileType) properly support the PR's type centralization and default file type resolution features.
43-43: LGTM: Consistent date formatting.Using
todayIsoDate()ensures consistent YYYY-MM-DD formatting across the codebase.
95-102: LGTM: Proper settings hierarchy for default file type.The implementation correctly resolves the default file extension following the appropriate precedence chain (collection → project → global → hardcoded default).
105-128: LGTM: Dynamic file extension throughout filename generation.The filename generation and conflict resolution properly use the computed
fileExtension, ensuring files are created with the correct extension based on project settings rather than a hardcoded value.src/hooks/mutations/useDeleteFileMutation.ts (1)
11-11: No issues found. Hook is not yet integrated into the codebase.The verification shows that
useDeleteFileMutationis not currently imported or used anywhere. Since there are no call sites, there are no instances to fix. The implementation itself is correct—fileIdis properly used for cache removal viaqueryKeys.fileContentat line 47. Once this hook is integrated into the codebase, TypeScript will enforce that all call sites provide the requiredfileIdfield.src/hooks/mutations/useRenameFileMutation.ts (1)
11-11: All call sites properly provide theoldFileIdfield.The verification confirms one production usage in
src/components/layout/LeftSidebar.tsx(line 203), which correctly suppliesoldFileId: file.idalong with all other required payload fields. Cache invalidation is properly implemented using the queryKey pattern as per coding guidelines.src/hooks/mutations/useSaveFileMutation.ts (1)
11-11: Verify whether call sites exist or if hook integration is incomplete.The
useSaveFileMutationhook has been added with thefileIdfield, but it is not currently imported or used anywhere in the codebase. All save functionality continues to route throughuseEditorStore().saveFile()in Zustand.The hook implements the TanStack Query cache invalidation pattern correctly; however, the breaking change you highlighted cannot be verified against actual call sites because none exist yet.
Confirm that:
- This hook is intended to replace the Zustand save logic and call sites will be updated in this or a subsequent commit
- Or if the hook is being added for future use, document its intended adoption path
src/hooks/queries/useFileContentQuery.ts (3)
3-6: LGTM! Type centralization improves organization.Moving
MarkdownContentto a dedicated types module and using type-only import syntax is good practice and aligns with the PR objective to centralize frontend types.
18-18: Helpful documentation added.This comment clarifies an important requirement for the Rust backend integration.
25-39: No issues found—cache invalidation properly implemented across all mutations.Verification confirms both concerns are correctly addressed:
Cache invalidation on rename (✓ verified): useRenameFileMutation.ts properly invalidates using
oldFileId, ensuring the fileId-based cache key is cleared even when the file path changes. No stale content risk.Caller compatibility (✓ verified): useEditorFileContent.ts has been updated to pass all required parameters, including
fileIdascurrentFile?.id || null.The design pattern is consistently applied: all mutations (save, delete, rename) and state changes that affect file content correctly invalidate the cache using fileId. This aligns with the coding guideline to invalidate TanStack Query caches on mutation success using the correct queryKey.
src/components/frontmatter/fields/StringField.tsx (1)
25-28: Selectors align with the direct store guidance.
Splitting value and updater into separate selectors keeps the component compliant with the direct store pattern and minimises needless re-renders.src/lib/files/sorting.test.ts (1)
84-185: Great coverage for date sorting edge cases.
Appreciate the assertions around undated entries, stability, and reference preservation; they give solid confidence in the helpers.src/lib/files/filtering.ts (1)
23-39: Draft filter extraction looks accurate.
This mirrors the prior LeftSidebar behaviour while keeping object references intact, so downstream consumers see consistent results.src/lib/files/filtering.test.ts (1)
23-110: Tests capture the combined isDraft/frontmatter scenarios well.
They ensure both flags are honoured, null mappings behave, and references stay stable, which matches the production logic.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
docs/tasks-done/task-2025-11-01-high-value-testing.md (2)
11-11: Consider using en-dashes for time ranges in formal documentation.The document uses hyphens for time ranges (e.g. "1-2 weeks", "2-3 days"), which is common in informal writing. For formal documentation, en-dashes (–) are conventionally preferred (e.g. "1–2 weeks", "2–3 days"). This improves typographical consistency and readability.
Example fix:
- **Total Time:** 1-2 weeks + **Total Time:** 1–2 weeks - **Time:** 2-3 days + **Time:** 2–3 daysApply this pattern across lines 11, 26, 122, 204, 245, and 474.
Also applies to: 26-26, 122-122, 204-204, 245-245, 474-474
201-238: Item 3 references Task 1 utilities but lacks specific test case details.This section appropriately directs implementers to the original task documentation for detailed test cases. However, embedding brief summary callouts here (e.g. prototype pollution risks for
setNestedValue) would improve self-containment and reduce the need to cross-reference external documentation.Consider adding a one-line note for each utility to highlight the most critical test scenario, for example:
- `setNestedValue` - nested paths, arrays, prototype pollution + `setNestedValue` - nested paths, arrays, **prototype pollution prevention**src/store/__tests__/storeQueryIntegration.test.ts (1)
351-371: Clarify test intent: saveFile always executes regardless of dirty state.The test name "should not save when not dirty" is misleading. The
saveFileimplementation doesn't checkisDirtybefore proceeding, so the save operation will always execute. The comment on lines 365-368 acknowledges this, but the test name creates confusion.Consider renaming to better reflect what's actually being tested:
- it('should not save when not dirty', async () => { + it('should maintain clean state when saving clean file', async () => {Alternatively, if the intent is to verify that saves don't occur when not dirty, the store implementation should be updated to include an early return:
saveFile: async (showToast = true) => { const { currentFile, isDirty } = get() if (!currentFile || !isDirty) return // ... rest of save logic }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/tasks-done/task-2025-11-01-high-value-testing.md(1 hunks)src/lib/schema.test.ts(1 hunks)src/store/__tests__/storeQueryIntegration.test.ts(1 hunks)src/store/sorting.test.ts(0 hunks)
💤 Files with no reviewable changes (1)
- src/store/sorting.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: Do not use React Hook Form; prefer Direct Store Pattern to avoid infinite loops
Use react-hotkeys-hook for keyboard shortcuts (e.g., mod+s, mod+1)
Type store destructuring explicitly when using Zustand stores
Files:
src/store/__tests__/storeQueryIntegration.test.tssrc/lib/schema.test.ts
src/store/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
In stores (no React hooks), dispatch DOM CustomEvents for cross-layer communication (Bridge Pattern)
Files:
src/store/__tests__/storeQueryIntegration.test.ts
{src/lib,src/hooks}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Invalidate TanStack Query caches on mutation success using the correct queryKey
Files:
src/lib/schema.test.ts
src/lib/**
📄 CodeRabbit inference engine (CLAUDE.md)
Extract complex, testable business logic (50+ lines or 2+ components) into src/lib/ modules
Files:
src/lib/schema.test.ts
src/lib/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Write unit tests for modules in src/lib/
Files:
src/lib/schema.test.ts
🧠 Learnings (5)
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Track progress by updating docs/tasks-todo after major work
Applied to files:
docs/tasks-done/task-2025-11-01-high-value-testing.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/hooks/**/*.test.{ts,tsx} : Write integration tests for hooks and workflows under src/hooks/
Applied to files:
docs/tasks-done/task-2025-11-01-high-value-testing.mdsrc/store/__tests__/storeQueryIntegration.test.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/lib/**/*.test.{ts,tsx} : Write unit tests for modules in src/lib/
Applied to files:
docs/tasks-done/task-2025-11-01-high-value-testing.mdsrc/store/__tests__/storeQueryIntegration.test.tssrc/lib/schema.test.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/components/**/*.test.{ts,tsx} : Write component tests for user interactions under src/components/
Applied to files:
docs/tasks-done/task-2025-11-01-high-value-testing.mdsrc/store/__tests__/storeQueryIntegration.test.ts
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to {src/lib,src/hooks}/**/*.{ts,tsx} : Invalidate TanStack Query caches on mutation success using the correct queryKey
Applied to files:
src/store/__tests__/storeQueryIntegration.test.ts
🧬 Code graph analysis (2)
src/store/__tests__/storeQueryIntegration.test.ts (3)
src/types/domain.ts (1)
FileEntry(14-38)src/store/editorStore.ts (1)
useEditorStore(40-301)src/test/mocks/toast.ts (1)
resetToastMocks(18-23)
src/lib/schema.test.ts (1)
src/lib/schema.ts (2)
CompleteSchema(2-5)deserializeCompleteSchema(86-121)
🪛 LanguageTool
docs/tasks-done/task-2025-11-01-high-value-testing.md
[typographical] ~11-~11: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...(remove orphaned test) Total Time: 1-2 weeks Why This Matters: - Schema p...
(HYPHEN_TO_EN)
[typographical] ~26-~26: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...hema.ts(currently untested) **Time:** 2-3 days ### Current State Thedeseriali...
(HYPHEN_TO_EN)
[typographical] ~122-~122: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...eeds complete data flow tests Time: 3-4 days ### Test Suites to Add #### A. F...
(HYPHEN_TO_EN)
[typographical] ~204-~204: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...om Task 1 Priority: HIGH Time: 1-2 days ### Utilities to Test Now that T...
(HYPHEN_TO_EN)
[typographical] ~245-~245: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...ultiple test files >300 lines Time: 2-3 days Note: This was Item 8 in the ...
(HYPHEN_TO_EN)
[grammar] ~473-~473: It appears that a pronoun is missing.
Context: ...n test:coverage ``` --- ## Notes - Depends on Task 1: Utilities extracted in Tas...
(SENT_START_DEPENDS)
[typographical] ~474-~474: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...1 need testing here - Total effort: 1-2 weeks - Critical for safety: Schema...
(HYPHEN_TO_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (17)
docs/tasks-done/task-2025-11-01-high-value-testing.md (7)
1-10: File location inconsistency: should this be intasks-todorather thantasks-done?The document is placed in
docs/tasks-done/but the status line (line 482) indicates "Ready for implementation", implying this is future work, not completed work. Based on the learnings that recommend tracking progress by updatingdocs/tasks-todoafter major work, this task definition should likely reside intasks-todo/instead.Please confirm the intended location and update the file path accordingly if this describes work to be done rather than work already completed.
21-115: Item 1 (Schema Parsing Tests) is well-structured with clear test patterns.The schema parsing test plan is comprehensive, including field type mapping, properties, error handling, and edge cases. The implementation pattern with concrete examples (lines 68–101) provides an excellent template for developers. The reference to existing test patterns is helpful.
117-198: Item 2 (File Workflow Integration Tests) provides good coverage of critical flows.The file workflow tests address loading, saving, and switching scenarios. The implementation notes correctly reference the existing mock patterns (
globalThis.mockTauri) and integration test setup, which should make implementation straightforward. Cross-references to existing tests ineditorStore.integration.test.tsare useful.
241-365: Item 4 (Decompose Long Test Files) has excellent decomposition strategy.The test file splitting strategy is clear, with specific file counts (552 lines, 452 lines), proposed splits, and verification steps. The section on "Before/After testing strategy" (lines 319–337) is particularly strong, providing concrete commands to validate that no tests are lost during refactoring.
367-379: Item 5 is appropriately brief for a straightforward cleanup task.Removing the orphaned test file is clearly justified and properly scoped.
382-467: Testing commands and key file references are well-organized.The commands section provides clear guidance for baseline, development, and completion phases. The file references align well with the learnings about test patterns in
src/hooks/**,src/lib/**, andsrc/components/**.
471-479: Notes section clearly articulates dependencies and success criteria.The notes correctly identify the dependency on Task 1, set realistic effort expectations (1–2 weeks), and emphasise the critical importance of schema parsing coverage. The coverage goal of >90% on critical paths is appropriate.
src/store/__tests__/storeQueryIntegration.test.ts (5)
1-25: LGTM! Well-structured mocks for integration testing.The mocking strategy appropriately isolates the store logic from external dependencies (query client, Tauri APIs, recovery functions) whilst allowing the test to focus on store-query integration behaviour.
26-33: LGTM! Standard test fixture.The mock file entry provides all necessary fields for testing file operations.
36-79: LGTM! Comprehensive test setup.The beforeEach hook properly resets all store state and mocks, ensuring test isolation. The inclusion of
lastSaveTimestamp: nullaligns with the new auto-save implementation.
81-296: LGTM! Comprehensive coverage of core store behaviours.The test suites thoroughly cover dirty state management, file switching, frontmatter updates (including dot notation and empty value removal), and file loading workflows. The tests properly verify state transitions and edge cases.
447-585: LGTM! Thorough coverage of file switching scenarios.The test suite comprehensively covers file switching behaviours, including state cleanup, dirty state isolation, auto-save timer management, and metadata preservation. The tests properly verify that no state leaks between files and that switching to the same file behaves correctly.
src/lib/schema.test.ts (5)
8-14: Excellent helper pattern for type narrowing.The
assertResulthelper using TypeScript's assertion signature is a clean approach to narrow the type fromCompleteSchema | nulltoCompleteSchema, making subsequent test assertions more readable and type-safe.
16-170: Comprehensive field type mapping coverage.The test suite thoroughly validates field type mapping from string representations to FieldType enum values, including all supported types, unknown type fallback, and complex array/reference subtypes.
172-369: Thorough field properties validation.The test suite comprehensively validates all field properties including required flags, optional metadata, enum values, various constraint types, reference fields, and nested field handling.
440-700: Excellent edge case coverage.The edge case tests are comprehensive, covering empty collections, deeply nested structures, backwards compatibility, and a valuable real-world integration test with a complex schema containing multiple field types, constraints, and relationships.
702-726: Good coverage of collection name handling.The tests validate that collection names are preserved correctly, including those with special characters (underscores, hyphens, numbers), ensuring no unintended sanitisation occurs.
…th resolution Complete Items 1-3 of Rust refactoring task: **Item 1: Simplify Brace Matching Logic (parser.rs)** - Extract `find_matching_closing_brace()` utility function - Refactor 3 functions to eliminate ~30 lines of duplication - Add 9 comprehensive tests for brace matching utility - Single source of truth for brace counting logic **Item 2: Extract Frontmatter Import Parsing Logic (files.rs)** - Extract 4 helper functions: `is_import_line()`, `has_import_terminator()`, `is_import_continuation()`, `should_skip_empty_line()` - Refactor `extract_imports_from_content()` for clarity - Add 4 comprehensive tests for import parsing helpers - 40% reduction in cyclomatic complexity **Item 3: Simplify Schema Field Path Resolution (parser.rs)** - Extract `find_immediate_field_name()` and `build_parent_path()` helpers - Simplify `resolve_field_path()` to orchestrate operations - Clearer separation between finding and building operations - All 11 existing field resolution tests pass **Test Coverage:** - Tests: 127 → 140 (+13 tests) - All tests passing ✅ - Code formatted and linted ✅ **Remaining:** Item 4 (Schema Merging) requires test expansion first (4 tests → 20+ tests) before refactoring per task requirements. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Complete Item 4 of Rust refactoring task: **Test Expansion (Test-First Approach)** - Added 10 comprehensive integration tests BEFORE refactoring - Tests cover: anyOf, arrays, enums, formats, nested objects, multiple types - Tests: 4 → 14 (3.5x improvement) - Lock in existing behavior with integration test coverage **Type Handler Extraction** - Extract `handle_anyof_type()` - dates, references, nullable unions - Extract `handle_array_type()` - arrays of primitives and objects - Extract `handle_object_type()` - nested and dynamic objects - Extract `handle_primitive_type()` - string, integer, number, boolean **Main Function Simplification** - Simplify `determine_field_type()` from 218 lines to ~84 lines - Dispatch to appropriate handlers based on schema structure - Maintain inline handling for special formats (email, url) - Clearer separation of concerns per type category **Benefits** - 60% reduction in function length (218 → 84 lines) - Each type handler is self-contained and testable - Easier to add new JSON Schema type support - Better error isolation (know which handler failed) - All 150 tests passing ✅ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
docs/tasks-todo/task-x-advanced-frontend-refactorings.md (4)
3-3: Format the bare URL as a Markdown link.To comply with Markdown standards, wrap the GitHub issue URL in angle brackets or convert it to a proper Markdown link format.
-https://github.com/dannysmith/astro-editor/issues/48 +<https://github.com/dannysmith/astro-editor/issues/48>Or, alternatively:
-https://github.com/dannysmith/astro-editor/issues/48 +[Issue #48](https://github.com/dannysmith/astro-editor/issues/48)
32-32: Replace hyphen with en-dash for time range.Time ranges should use an en-dash (–) rather than a hyphen (-) for clarity and correct typographical style.
-**Time:** 5-7 days +**Time:** 5–7 days
365-365: Replace hyphen with en-dash for time range.Consistent with standard typographical conventions for ranges.
-**Total effort:** 5-7 days (~1 week) +**Total effort:** 5–7 days (~1 week)
75-237: Add import context to TypeScript code examples for clarity.The code examples reference types and functions (e.g.,
Text,Decoration,Linefrom CodeMirror;DecorationSetfrom the extension API) without explicit imports. Whilst acceptable for planning documentation, consider adding a comment block above the first code example noting the expected imports to make the guidance more immediately actionable for implementers.+// Note: Code examples below assume the following imports: +// import { Text } from '@codemirror/state' +// import { Decoration, DecorationSet } from '@codemirror/view' +// and types from your codebase (CopyeditSettings, etc.) + function validateMatch(docs/tasks-todo/task-x-event-bridge-refactor.md (3)
1-1509: Consider consolidating overlapping architectural sections for better readability.The document covers the same architectural decisions (Hybrid Action Hooks vs. Callback Registry) multiple times:
- Lines 85–183: Initial recommendation
- Lines 293–369: Expert consultation analysis
- Lines 703–718: Technical comparison matrix
- Lines 1049–1126: Performance analysis
For a task-planning document, this creates maintenance burden and makes navigation difficult. The current structure reads like concatenated analysis reports rather than a coherent plan.
Recommendation: Restructure as:
- Executive Summary (1 page): Decision + key reasoning
- Implementation Plan (1 page): Phased approach + success criteria
- Appendix (optional separate file): Detailed research, expert consultation, concerns analysis
This keeps the task document actionable whilst preserving detailed analysis for reference.
3-3: Wrap bare URL in Markdown link syntax.The URL should be formatted as a link:
- [Issue #49](https://github.com/dannysmith/astro-editor/issues/49) + [Issue #49](https://github.com/dannysmith/astro-editor/issues/49)This improves readability and follows Markdown best practices.
479-1328: Standardise heading hierarchy to improve document structure.The document inconsistently uses bold emphasis (
**Title**) instead of Markdown headings (### Title) for subsections. This affects readability and prevents proper table of contents generation.Lines with emphasis-as-heading issues: 479, 499, 509, 1149, 1182, 1203, 1239, 1251, 1264, 1317, 1328 (and likely others).
Replace patterns like:
- **Footgun #1: Forgetting to Wire Callback in Layout** + ### Footgun #1: Forgetting to Wire Callback in LayoutThis improves document navigation and consistency with Markdown standards.
docs/tasks-todo/task-x-god-hook.md (1)
3-3: Wrap bare URL in Markdown link syntax.Format as:
[Issue #50](https://github.com/dannysmith/astro-editor/issues/50)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/tasks-done/task-2025-11-01-rust-refactorings.md(1 hunks)docs/tasks-todo/task-x-advanced-frontend-refactorings.md(1 hunks)docs/tasks-todo/task-x-event-bridge-refactor.md(1 hunks)docs/tasks-todo/task-x-god-hook.md(1 hunks)src-tauri/src/commands/files.rs(17 hunks)src-tauri/src/parser.rs(8 hunks)src-tauri/src/schema_merger.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src-tauri/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use modern Rust formatting: format("{variable}")
Files:
src-tauri/src/parser.rssrc-tauri/src/commands/files.rssrc-tauri/src/schema_merger.rs
🧠 Learnings (7)
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Track progress by updating docs/tasks-todo after major work
Applied to files:
docs/tasks-done/task-2025-11-01-rust-refactorings.mddocs/tasks-todo/task-x-event-bridge-refactor.mddocs/tasks-todo/task-x-god-hook.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to docs/developer/** : Update docs/developer/ guides when introducing new patterns
Applied to files:
docs/tasks-todo/task-x-event-bridge-refactor.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/store/**/*.ts : In stores (no React hooks), dispatch DOM CustomEvents for cross-layer communication (Bridge Pattern)
Applied to files:
docs/tasks-todo/task-x-event-bridge-refactor.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/components/**/*.tsx : Components should access stores directly (Direct Store Pattern) instead of prop callbacks that cause dependency loops
Applied to files:
docs/tasks-todo/task-x-event-bridge-refactor.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/**/*.{ts,tsx} : Use react-hotkeys-hook for keyboard shortcuts (e.g., mod+s, mod+1)
Applied to files:
docs/tasks-todo/task-x-god-hook.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/hooks/**/*.ts : Place reusable React hooks and side-effect logic in src/hooks/
Applied to files:
docs/tasks-todo/task-x-god-hook.md
📚 Learning: 2025-10-23T17:33:56.867Z
Learnt from: CR
Repo: dannysmith/astro-editor PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-23T17:33:56.867Z
Learning: Applies to src/components/layout/Layout.tsx : Add event listeners related to command palette behavior in Layout.tsx when needed
Applied to files:
docs/tasks-todo/task-x-god-hook.md
🧬 Code graph analysis (1)
src-tauri/src/commands/files.rs (1)
src-tauri/src/commands/mdx_components.rs (1)
validate_project_path(17-32)
🪛 LanguageTool
docs/tasks-done/task-2025-11-01-rust-refactorings.md
[typographical] ~7-~7: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...schema merger modules. Total Time: 2-3 weeks (can be done incrementally) **Im...
(HYPHEN_TO_EN)
[typographical] ~51-~51: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...s (146-206, 294-335, 338-368) Time: 2-3 days Risk: LOW ### Current Issues ...
(HYPHEN_TO_EN)
[typographical] ~201-~201: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...ports_from_content` function) Time: 2-3 days Risk: LOW (comprehensive tests...
(HYPHEN_TO_EN)
[style] ~273-~273: ‘overall structure’ might be wordy. Consider a shorter alternative.
Context: ...ld_skip_empty_line()` - Preserve the overall structure: outer loop → import detection → multi-...
(EN_WORDINESS_PREMIUM_OVERALL_STRUCTURE)
[typographical] ~386-~386: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...resolve_field_path` function) Time: 2-3 days Risk: LOW-MEDIUM (good test co...
(HYPHEN_TO_EN)
[uncategorized] ~438-~438: Possible missing preposition found.
Context: ...uild_parent_path()` to get parents - Combine into complete path - Keep existing e...
(AI_HYDRA_LEO_MISSING_TO)
[typographical] ~537-~537: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...iple long functions (260-616) Time: 4-5 days Risk: MEDIUM (core schema proc...
(HYPHEN_TO_EN)
[typographical] ~552-~552: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...determinations - Each type variation is 15-30 lines of nearly identical code - Diffic...
(HYPHEN_TO_EN)
[typographical] ~691-~691: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...ust pass) - Run new handler unit tests (10-15 tests must pass) - Test with complex re...
(HYPHEN_TO_EN)
[typographical] ~799-~799: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...on for other parser work - Time: 2-3 days - Milestone: `cargo test pa...
(HYPHEN_TO_EN)
[typographical] ~803-~803: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...dent, reduces complexity - Time: 2-3 days - Milestone: `cargo test ex...
(HYPHEN_TO_EN)
[typographical] ~807-~807: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...tion) - Builds on Item 1 - Time: 2-3 days - Milestone: `cargo test re...
(HYPHEN_TO_EN)
[typographical] ~811-~811: If specifying a range, consider using an en dash instead of a hyphen.
Context: ... experience with first 3 - Time: 4-5 days - Critical: ADD comprehensi...
(HYPHEN_TO_EN)
[typographical] ~819-~819: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...sts --- ## Notes - Total effort: 2-3 weeks (can be done incrementally) - **E...
(HYPHEN_TO_EN)
docs/tasks-todo/task-x-event-bridge-refactor.md
[typographical] ~225-~225: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...ode for schema-field-order Effort: 2-3 hours ### Phase 2: Apply Pattern to Ot...
(HYPHEN_TO_EN)
[typographical] ~233-~233: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...ther orchestration actions Effort: 1-2 hours each ### Phase 3: Clean Up 1. R...
(HYPHEN_TO_EN)
[typographical] ~241-~241: If specifying a range, consider using an en dash instead of a hyphen.
Context: ...oks and store interactions Effort: 1-2 hours Total: ~1 day ## Assessment...
(HYPHEN_TO_EN)
[style] ~249-~249: Consider using a different verb for a more formal wording.
Context: ...erience and maintainability but doesn't fix any user-facing issues. ## References ...
(FIX_RESOLVE)
[style] ~274-~274: This phrase is redundant. Consider writing “eliminate”.
Context: ...ry data Key Finding: Both patterns completely eliminate the technical debt of the current event...
(COMPLETELY_ANNIHILATE)
[misspelling] ~409-~409: This word is normally spelled as one.
Context: ...oordination would be an architectural anti-pattern: - Tauri events are for **native-fro...
(EN_COMPOUNDS_ANTI_PATTERN)
[uncategorized] ~481-~481: Possible missing article found.
Context: ...* Store has all context needed to make single, well-formed Tauri command call: ```ty...
(AI_HYDRA_LEO_MISSING_A)
[style] ~656-~656: ‘at a fraction of’ might be wordy. Consider a shorter alternative.
Context: ... This gives "90% of Redux's superpowers at a fraction of the code, bundle size, and cognitive lo...
(EN_WORDINESS_PREMIUM_AT_A_FRACTION_OF)
[grammar] ~800-~800: A determiner may be missing.
Context: ...atterns over custom abstractions ✅ Want lowest learning curve for new developers ✅ Val...
(THE_SUPERLATIVE)
[typographical] ~816-~816: Use a comma after an introductory phrase.
Context: ...lex desktop apps, multi-window UIs ### For This Codebase Specifically Context: - ...
(COMMA_INTRODUCTORY_WORDS_PHRASES)
[grammar] ~1233-~1233: The verb ‘refactor’ is plural. Did you mean: “refactors”? Did you use a verb instead of a noun?
Context: ...- but current code has same issue! This refactor makes it easier to fix because logi...
(PLURAL_VERB_AFTER_THIS)
[style] ~1473-~1473: To elevate your writing, try using a synonym here.
Context: ...rise - [ ] Team finds pattern confusing/hard to use - [ ] Testing becomes significan...
(HARD_TO)
[grammar] ~1499-~1499: The verb ‘refactor’ is plural. Did you mean: “refactors”? Did you use a verb instead of a noun?
Context: ... --- ## Confidence Level: HIGH ✅ This refactor: - Eliminates technical debt (polling,...
(PLURAL_VERB_AFTER_THIS)
docs/tasks-todo/task-x-advanced-frontend-refactorings.md
[typographical] ~32-~32: If specifying a range, consider using an en dash instead of a hyphen.
Context: ... handles all POS highlighting Time: 5-7 days Risk: MEDIUM (user-facing feat...
(HYPHEN_TO_EN)
[typographical] ~365-~365: If specifying a range, consider using an en dash instead of a hyphen.
Context: ....md --- ## Notes - Total effort: 5-7 days (~1 week) - **User-facing feature:...
(HYPHEN_TO_EN)
docs/tasks-todo/task-x-god-hook.md
[misspelling] ~58-~58: This word is normally spelled as one.
Context: ...Rust Toast Bridge: - Initializes bi-directional toast communication from Rust backend ...
(EN_COMPOUNDS_BI_DIRECTIONAL)
[uncategorized] ~85-~85: A comma may be missing after the conjunctive/linking adverb ‘Currently’.
Context: ...e-runs whenever any dependency changes. Currently has dependencies on `createNewFileWithQ...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[style] ~106-~106: Consider using a different verb for a more formal wording.
Context: ...intainability and testability but won't fix any user-facing problems. The criticali...
(FIX_RESOLVE)
🪛 markdownlint-cli2 (0.18.1)
docs/tasks-todo/task-x-event-bridge-refactor.md
3-3: Bare URL used
(MD034, no-bare-urls)
479-479: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
499-499: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
509-509: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
612-612: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
1149-1149: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1182-1182: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1203-1203: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1239-1239: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1251-1251: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1264-1264: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1317-1317: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
1328-1328: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
docs/tasks-todo/task-x-advanced-frontend-refactorings.md
3-3: Bare URL used
(MD034, no-bare-urls)
docs/tasks-todo/task-x-god-hook.md
3-3: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (2)
docs/tasks-todo/task-x-event-bridge-refactor.md (1)
85-243: Strong architectural analysis and actionable implementation plan.The Hybrid Action Hooks recommendation is well-supported by:
- ✅ Clear decision criteria (lines 793–849)
- ✅ Detailed performance analysis (lines 1049–1127)
- ✅ Footgun comparison (lines 1145–1290)
- ✅ Phased implementation with gates (lines 1427–1475)
- ✅ Success and abort criteria (lines 1455–1475)
The document successfully argues why this refactor improves maintainability and developer experience. The incremental Phase 1 approach (proof of concept on
saveFile) makes this low-risk.Ready for implementation planning. Once restructured, this will serve as a solid blueprint for the refactor.
docs/tasks-todo/task-x-god-hook.md (1)
1-236: Well-scoped and actionable task planning document.This document clearly defines:
- ✅ Current monolith: 486 lines, 8 disparate concerns
- ✅ Proposed decomposition: 6–7 focused hooks with single responsibilities
- ✅ Concrete hook names and responsibilities (lines 114–175)
- ✅ Duplication issues to address during refactor (lines 204–227)
- ✅ Measurable success criteria (lines 229–237)
The document appropriately acknowledges this is a maintenance improvement, not a bug fix. The connection to
task-x-event-bridge-refactor.mdis well-articulated (line 200–202).Ready for implementation planning.
1 participant
Various internal code improvements based on a series of AI reviews of the codebase.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests