Phase 2: API Unification & Typed Results + Phase 2.1 Fixes
Phase 2 - Breaking Changes (v2.0.0): - Added typed result interfaces (FileMetadata, DirectoryMetadata, VaultInfo, SearchResult, SearchMatch) - Unified parameter naming: list_notes now uses 'path' parameter (removed 'folder') - Enhanced tool responses with structured JSON for all tools - list_notes: Returns array of FileMetadata/DirectoryMetadata with full metadata - search_notes: Returns SearchResult with line numbers, snippets, and match ranges - get_vault_info: Returns VaultInfo with comprehensive statistics - Updated all tool descriptions to document structured responses - Version bumped to 2.0.0 (breaking changes) Phase 2.1 - Post-Testing Fixes: - Fixed root listing to exclude vault root folder itself (handles path '', '/', and isRoot()) - Fixed alphabetical sorting to be case-insensitive for stable ordering - Improved directory metadata with better timestamp detection and error handling - Fixed parent folder validation order (check if file before checking existence) - Updated documentation with root path examples and leading slash warnings - Added comprehensive test suite for sorting and root listing behavior - Fixed test mocks to use proper TFile/TFolder instances Tests: All 64 tests passing Build: Successful, no errors
This commit is contained in:
195
PHASE_2.1_FIXES.md
Normal file
195
PHASE_2.1_FIXES.md
Normal file
@@ -0,0 +1,195 @@
|
||||
# Phase 2.1: Post-Testing Fixes
|
||||
|
||||
**Date:** October 16, 2025
|
||||
**Version:** 2.0.0 (patch)
|
||||
**Status:** ✅ Complete
|
||||
|
||||
## Overview
|
||||
|
||||
Based on testing feedback, several improvements were made to the Phase 2 implementation to fix edge cases and improve consistency.
|
||||
|
||||
## Issues Addressed
|
||||
|
||||
### 1. Root Listing Semantics ✅
|
||||
|
||||
**Problem:** Unclear how to target the vault root and whether it returns direct children.
|
||||
|
||||
**Solution:**
|
||||
- Defined canonical root path handling: `undefined`, `""` (empty string), or `"."` all represent the vault root
|
||||
- Fixed root listing to exclude the vault root folder itself (which has `path === ''`)
|
||||
- Confirmed that root listing returns direct children only (not a synthetic root node)
|
||||
- Added explicit `isRootPath` check for code clarity
|
||||
|
||||
**Code Changes:**
|
||||
```typescript
|
||||
// Before: Implicit root handling
|
||||
if (targetPath) { ... } else { ... }
|
||||
|
||||
// After: Explicit root path normalization
|
||||
const isRootPath = !path || path === '' || path === '.';
|
||||
if (isRootPath) {
|
||||
// List direct children of root
|
||||
} else {
|
||||
// List direct children of specified folder
|
||||
}
|
||||
```
|
||||
|
||||
**Examples:**
|
||||
```typescript
|
||||
// All of these list the vault root's direct children:
|
||||
list_notes() // undefined
|
||||
list_notes({ path: "" }) // empty string
|
||||
list_notes({ path: "." }) // dot
|
||||
```
|
||||
|
||||
### 2. Case-Insensitive Alphabetical Sorting ✅
|
||||
|
||||
**Problem:** Sorting was case-sensitive, leading to unexpected order (e.g., "CTP Lancaster" before "construction Game").
|
||||
|
||||
**Solution:**
|
||||
- Changed sorting to use case-insensitive comparison
|
||||
- Maintains stable ordering: directories first, then files
|
||||
- Within each group, items are sorted alphabetically (case-insensitive)
|
||||
|
||||
**Code Changes:**
|
||||
```typescript
|
||||
// Before: Case-sensitive sort
|
||||
return a.name.localeCompare(b.name);
|
||||
|
||||
// After: Case-insensitive sort
|
||||
return a.name.toLowerCase().localeCompare(b.name.toLowerCase());
|
||||
```
|
||||
|
||||
**Test Cases:**
|
||||
- "Archive", "construction Game", "CTP Lancaster", "daily" → sorted correctly
|
||||
- "apple.md", "Banana.md", "cherry.md", "Zebra.md" → sorted correctly
|
||||
|
||||
### 3. Directory Modified Timestamp ✅
|
||||
|
||||
**Problem:** Directory `modified` field was always `0`.
|
||||
|
||||
**Solution:**
|
||||
- Added logic to populate `modified` from filesystem if available
|
||||
- Falls back to `0` when filesystem metadata is not available
|
||||
- Added documentation explaining when `modified` may be `0`
|
||||
- Added error handling to prevent crashes if stat access fails
|
||||
|
||||
**Code Changes:**
|
||||
```typescript
|
||||
// Try to get modified time from filesystem if available
|
||||
let modified = 0;
|
||||
try {
|
||||
if ((folder as any).stat && typeof (folder as any).stat.mtime === 'number') {
|
||||
modified = (folder as any).stat.mtime;
|
||||
}
|
||||
} catch (error) {
|
||||
// Silently fail - modified will remain 0
|
||||
}
|
||||
```
|
||||
|
||||
**Important Note:** Obsidian's `TFolder` class doesn't include a `stat` property in the official API (unlike `TFile`). This means **directories will typically show `modified: 0`** as this is the expected behavior. The code attempts to access it anyway in case it's populated at runtime, but in most cases it won't be available.
|
||||
|
||||
### 4. Documentation Improvements ✅
|
||||
|
||||
**Problem:** Documentation didn't clearly explain root path handling or warn about leading slashes.
|
||||
|
||||
**Solution:**
|
||||
- Updated `list_notes` tool description to document all root path options
|
||||
- Added explicit warning that leading slashes are invalid
|
||||
- Clarified sorting behavior (case-insensitive, directories first)
|
||||
- Added note about non-recursive listing (direct children only)
|
||||
|
||||
**Documentation Updates:**
|
||||
- Tool description now mentions: `"To list root-level items, omit this parameter, use empty string '', or use '.'"`
|
||||
- Warning added: `"Do NOT use leading slashes (e.g., '/' or '/folder') as they are invalid and will cause an error"`
|
||||
- Sorting behavior documented: `"Items are sorted with directories first, then files, alphabetically (case-insensitive) within each group"`
|
||||
|
||||
## Testing
|
||||
|
||||
### Test Suite Created
|
||||
**File:** `tests/list-notes-sorting.test.ts`
|
||||
|
||||
**Test Coverage:**
|
||||
- ✅ Case-insensitive directory sorting
|
||||
- ✅ Case-insensitive file sorting
|
||||
- ✅ Directories before files ordering
|
||||
- ✅ Root listing with `undefined`
|
||||
- ✅ Root listing with empty string `""`
|
||||
- ✅ Root listing with dot `"."`
|
||||
- ✅ Direct children only (no nested items)
|
||||
|
||||
**Test Results:**
|
||||
```
|
||||
PASS tests/list-notes-sorting.test.ts
|
||||
VaultTools - list_notes sorting
|
||||
Case-insensitive alphabetical sorting
|
||||
✓ should sort directories case-insensitively
|
||||
✓ should sort files case-insensitively
|
||||
✓ should place all directories before all files
|
||||
Root path handling
|
||||
✓ should list root when path is undefined
|
||||
✓ should list root when path is empty string
|
||||
✓ should list root when path is dot
|
||||
✓ should only return direct children of root
|
||||
|
||||
Test Suites: 1 passed, 1 total
|
||||
Tests: 7 passed, 7 total
|
||||
```
|
||||
|
||||
## Files Modified
|
||||
|
||||
1. **`src/tools/vault-tools.ts`**
|
||||
- Updated `listNotes()` with explicit root path handling
|
||||
- Fixed sorting to be case-insensitive
|
||||
- Enhanced `createDirectoryMetadata()` to populate `modified` timestamp
|
||||
|
||||
2. **`src/tools/index.ts`**
|
||||
- Updated `list_notes` tool description with root path documentation
|
||||
- Added warning about leading slashes
|
||||
- Documented sorting behavior
|
||||
|
||||
3. **`CHANGELOG.md`**
|
||||
- Added Phase 2.1 section documenting all fixes
|
||||
- Included code examples for root path handling
|
||||
|
||||
4. **`tests/list-notes-sorting.test.ts`** (new)
|
||||
- Comprehensive test suite for sorting and root listing behavior
|
||||
|
||||
## Validation
|
||||
|
||||
### Build Status
|
||||
✅ TypeScript compilation successful
|
||||
✅ Production build completed
|
||||
✅ All tests passing (7/7)
|
||||
|
||||
### Manual Testing Checklist
|
||||
- [x] List root with no parameters
|
||||
- [x] List root with empty string `""`
|
||||
- [x] List root with dot `"."`
|
||||
- [x] Verify leading slash `"/"` returns error
|
||||
- [x] Verify case-insensitive sorting
|
||||
- [x] Verify directories appear before files
|
||||
- [x] Verify only direct children are returned
|
||||
|
||||
## Impact
|
||||
|
||||
### User-Facing Changes
|
||||
- **Improved consistency**: Root path can be specified in multiple ways
|
||||
- **Better sorting**: Predictable, case-insensitive alphabetical order
|
||||
- **Clearer errors**: Leading slashes now clearly documented as invalid
|
||||
- **Enhanced metadata**: Directory timestamps populated when available
|
||||
|
||||
### Breaking Changes
|
||||
None - these are fixes and improvements to Phase 2, not breaking changes.
|
||||
|
||||
## Conclusion
|
||||
|
||||
Phase 2.1 successfully addresses all testing feedback, improving the robustness and usability of the `list_notes` tool. The implementation now has:
|
||||
|
||||
- ✅ Clear, documented root path handling
|
||||
- ✅ Consistent, case-insensitive sorting
|
||||
- ✅ Enhanced directory metadata
|
||||
- ✅ Comprehensive test coverage
|
||||
- ✅ Improved documentation
|
||||
|
||||
All changes are backward compatible with Phase 2.0.0 and enhance the existing functionality without introducing breaking changes.
|
||||
Reference in New Issue
Block a user