Skip to content

fix(gateway): handle brackets in JSON strings - #1594

Merged
steebchen merged 1 commit into
mainfrom
investigate-attachment
Feb 4, 2026
Merged

steebchen merged 1 commit into
mainfrom
investigate-attachment

Conversation

@steebchen

@steebchen steebchen commented Feb 4, 2026

Copy link
Copy Markdown
Member

Summary

  • Fixed mightBeCompleteJson heuristic to skip brackets/braces inside JSON string values
  • Added 17 unit tests including the production failure case

The heuristic was incorrectly counting brackets inside strings (e.g., "delta":"\"]\"") as unbalanced, causing valid JSON to be marked as incomplete. This led to SSE events being concatenated together during streaming, resulting in JSON parse errors.

Test plan

  • Unit tests pass (pnpm test:unit)
  • Build succeeds (pnpm build)
  • Format check passes (pnpm format)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved JSON validation to correctly identify complete and incomplete JSON while properly handling strings containing special characters and escaped quotes.
  • Tests

    • Added comprehensive unit test suite covering multiple scenarios: simple objects and arrays, incomplete JSON, empty and non-JSON strings, strings with special characters, unclosed strings, nested structures, arrays of objects, and complex edge cases.

The mightBeCompleteJson heuristic was incorrectly counting brackets
and braces inside JSON string values, causing valid JSON to be
marked as incomplete. This led to SSE events being concatenated
together, resulting in JSON parse errors during streaming.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 4, 2026 18:57
@steebchen
steebchen enabled auto-merge February 4, 2026 18:57
@coderabbitai

coderabbitai Bot commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Updates the mightBeCompleteJson utility function to properly handle JSON strings by tracking state and skipping escaped characters, replacing a naive brace/bracket counting approach. Comprehensive unit tests are added to validate behavior across various edge cases.

Changes

Cohort / File(s) Summary
JSON Completion Validation
apps/gateway/src/chat/tools/might-be-complete-json.ts, apps/gateway/src/chat/tools/might-be-complete-json.spec.ts
Implementation refactored to use string-aware traversal with inString state tracking and escaped quote handling; comprehensive test suite added covering simple structures, incomplete JSON, unclosed strings, nested objects/arrays, and edge cases with brackets/braces inside string literals.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

auto-merge

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: fixing a bug where brackets inside JSON strings are incorrectly counted, causing valid JSON to be marked incomplete.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch investigate-attachment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens the mightBeCompleteJson heuristic used in SSE parsing so that it no longer misclassifies valid JSON as incomplete when brackets or braces appear inside string values, and adds targeted unit tests around this behavior.

Changes:

  • Updated mightBeCompleteJson to scan the JSON string character-by-character, tracking whether the cursor is inside a string and ignoring brackets/braces there, while handling escaped characters.
  • Added a dedicated might-be-complete-json.spec.ts test suite with cases for simple structures, unbalanced input, escaped characters, and the real-world production failure scenario.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
apps/gateway/src/chat/tools/might-be-complete-json.ts Refines the JSON completeness heuristic to skip bracket/brace counting inside string literals and to treat unterminated strings as incomplete.
apps/gateway/src/chat/tools/might-be-complete-json.spec.ts Introduces comprehensive unit tests validating the heuristic across normal, edge, and production-derived JSON examples.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@steebchen
steebchen added this pull request to the merge queue Feb 4, 2026
Merged via the queue into main with commit 0fcb06e Feb 4, 2026
19 checks passed
@steebchen
steebchen deleted the investigate-attachment branch February 4, 2026 19:12
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.

2 participants