Skip to content

Fix calendar synthesis JSON parsing#5760

Merged
kodjima33 merged 1 commit intomainfrom
fix/calendar-reader-json-parse
Mar 17, 2026
Merged

Fix calendar synthesis JSON parsing#5760
kodjima33 merged 1 commit intomainfrom
fix/calendar-reader-json-parse

Conversation

@kodjima33
Copy link
Copy Markdown
Collaborator

Summary

  • LLM (ACPBridge) sometimes returns JSON wrapped in ```json code fences or with leading text
  • This caused "Failed to parse synthesis response" even when events were fetched successfully
  • Fix: strip markdown code fences, find first { character, trim before parsing

🤖 Generated with Claude Code

The ACPBridge LLM sometimes returns JSON wrapped in markdown code fences
or with leading text. Strip code fences and find the JSON object before
parsing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@kodjima33 kodjima33 merged commit dfd35a4 into main Mar 17, 2026
@kodjima33 kodjima33 deleted the fix/calendar-reader-json-parse branch March 17, 2026 21:52
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 17, 2026

Greptile Summary

This PR fixes a parsing failure in CalendarReaderService.synthesizeFromEvents where the ACPBridge LLM occasionally wraps its JSON response in Markdown code fences (```json or ```) or prepends explanatory text, causing JSONSerialization to fail even though a valid JSON payload was present.

Changes made:

  • responseText is changed from let to var so it can be mutated in-place.
  • A two-branch stripping pass is added: first looking for a ```json fence, then a plain ``` fence; each branch slices the inner content out correctly.
  • A final firstIndex(of: "{") search trims any remaining leading text before the JSON object.
  • Trailing whitespace is stripped with trimmingCharacters.

Issue found:

  • The firstIndex(of: "{") path (lines 180–182) only removes content before the first {; it does not strip anything that comes after the closing }. If the LLM appends trailing prose outside a code fence (e.g., a follow-up sentence), JSONSerialization will still fail. Pairing firstIndex(of: "{") with lastIndex(of: "}") would close this gap without any added complexity.

Confidence Score: 3/5

  • Safe to merge with the caveat that the trailing-text edge case is unaddressed; real-world impact depends on how often the LLM adds postamble text without code fences.
  • The code-fence stripping branches are correct and the main regression (fences causing parse failures) is fixed. The only remaining gap is that the firstIndex(of: "{") guard does not remove trailing non-JSON text, which could still cause failures in a subset of LLM responses. The fix is a one-liner, so the score is 3 rather than lower.
  • desktop/Desktop/Sources/CalendarReaderService.swift — specifically lines 180–182 where trailing text after the JSON object is not stripped.

Important Files Changed

Filename Overview
desktop/Desktop/Sources/CalendarReaderService.swift Adds JSON extraction heuristics (code-fence stripping + first-{ seeking) to handle LLM responses that don't return raw JSON. The code-fence paths are correct; however the braceStart path only trims leading text and leaves any trailing prose, so responses with both a preamble and postamble would still fail to parse.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[LLM returns result.text] --> B{Contains backtick-json fence?}
    B -- Yes --> C[Strip everything before fence opening\nSlice from upperBound]
    C --> D{Closing fence found?}
    D -- Yes --> E[Slice up to closing fence]
    D -- No --> F[Keep remainder as-is]
    E --> G
    F --> G
    B -- No --> H{Contains plain backtick fence?}
    H -- Yes --> I[Strip everything before fence opening]
    I --> J{Closing fence found?}
    J -- Yes --> K[Slice up to closing fence]
    J -- No --> L[Keep remainder as-is]
    K --> G
    L --> G
    H -- No --> G
    G{First curly brace found?}
    G -- Yes --> M[Slice from first brace onward\n⚠️ trailing text NOT removed]
    G -- No --> N[responseText unchanged]
    M --> O[Trim whitespace]
    N --> O
    O --> P{JSONSerialization succeeds?}
    P -- Yes --> Q[Parse memories, tasks, profile]
    P -- No --> R[Log failure, return 0,0,empty]
Loading

Last reviewed commit: 9c975c8

Comment on lines +180 to +182
if let braceStart = responseText.firstIndex(of: "{") {
responseText = String(responseText[braceStart...])
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Trailing text not stripped — parsing can still fail

The braceStart approach correctly removes content before the first {, but leaves anything after the closing } intact. If the LLM appends trailing prose (e.g., \n\nLet me know if you need more info.) after the JSON object and outside of any code fence, JSONSerialization will still fail because of the unexpected trailing text.

Since the PR description explicitly mentions fixing the "leading text" case, this gap should also be closed. A simple, more robust approach is to pair firstIndex(of: "{") with lastIndex(of: "}"):

Suggested change
if let braceStart = responseText.firstIndex(of: "{") {
responseText = String(responseText[braceStart...])
}
if let braceStart = responseText.firstIndex(of: "{"),
let braceEnd = responseText.lastIndex(of: "}"),
braceStart <= braceEnd {
responseText = String(responseText[braceStart...braceEnd])
}

This handles responses that have both a preamble and a postamble around the raw JSON object.

Glucksberg pushed a commit to Glucksberg/omi-local that referenced this pull request Apr 28, 2026
## Summary
- LLM (ACPBridge) sometimes returns JSON wrapped in ```json code fences
or with leading text
- This caused "Failed to parse synthesis response" even when events were
fetched successfully
- Fix: strip markdown code fences, find first `{` character, trim before
parsing

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant