-
-
Notifications
You must be signed in to change notification settings - Fork 407
feat: Update file cache to use markdown instead of json #1572
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
logancyang
commented
Jun 24, 2025
- Added user_id to BrevilabsClient for backend rate limiting.
- Updated FileCache to support markdown file format and migrate legacy JSON cache files.
- Improved cache management by ensuring both markdown and legacy files are handled appropriately during read and write operations.
- Added user_id to BrevilabsClient for backend rate limiting. - Updated FileCache to support markdown file format and migrate legacy JSON cache files. - Improved cache management by ensuring both markdown and legacy files are handled appropriately during read and write operations.
bugbot run |
…arsing - Removed user_id field from BrevilabsClient to streamline request handling. - Updated FileCache to parse content based on file format, supporting both JSON and string types for improved cache management. - Enhanced serialization logic for writing content to file cache, ensuring proper handling of different data types.
bugbot run |
- Removed the legacy cache path and migration logic from FileCache, streamlining the cache retrieval process. - Enhanced content parsing to differentiate between JSON and string formats based on content structure. - Updated cache entry creation to ensure proper timestamp handling without legacy file dependencies.
bugbot run |
bugbot run |
…andling - Introduced safeParseJSON method for robust JSON parsing. - Updated cache retrieval logic to handle metadata in markdown files, differentiating between JSON and string content. - Ensured proper timestamp preservation during cache entry creation and file writing.
bugbot run |
…ing content handling - Eliminated the safeParseJSON method to enhance clarity in JSON parsing. - Updated cache retrieval logic to directly handle JSON and string content based on structure. - Simplified file writing process by removing metadata header, focusing on efficient content serialization.
bugbot run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: FileCache JSON Handling Fails
The FileCache
's get
method, when reading .md
cache files, introduces type safety and data integrity issues:
- Incorrect JSON handling: The heuristic for detecting JSON content is flawed. It fails to parse JSON-serialized primitive types (e.g., numbers, booleans), which are stored as strings but retrieved as plain strings. Conversely, it incorrectly parses string content that coincidentally resembles JSON, returning an object/array instead of the original string.
- Unsafe type casting: If JSON parsing fails or isn't attempted, the string content read from the file is unsafely cast directly to the generic type
T
, leading to runtime type mismatches whenT
is notstring
. - Inconsistent content handling: The JSON detection uses trimmed content, but the subsequent parsing uses untrimmed content.
src/cache/fileCache.ts#L61-L77
obsidian-copilot/src/cache/fileCache.ts
Lines 61 to 77 in c220e9a
// Try to parse as JSON first (for non-string types that were serialized) | |
const trimmedContent = cacheContent.trim(); | |
if ( | |
(trimmedContent.startsWith("{") && trimmedContent.endsWith("}")) || | |
(trimmedContent.startsWith("[") && trimmedContent.endsWith("]")) | |
) { | |
try { | |
parsedContent = JSON.parse(cacheContent); | |
} catch { | |
// JSON parsing failed, treat as string content | |
parsedContent = cacheContent as T; | |
} | |
} else { | |
// Plain text content (primary case for markdown) | |
parsedContent = cacheContent as T; | |
} |
Was this report helpful? Give feedback by reacting with 👍 or 👎