Repository navigation
Conversation
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
There was a problem hiding this comment.
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/partial_json_parser/core/complete.py" line_range="21" />
<code_context>
try:
- result = complete_any(json_string.strip(), allow, is_top_level)
+ # Trailing whitespace can belong to an unfinished string.
+ result = complete_any(json_string.lstrip(), allow, is_top_level)
if result is False:
raise PartialJSON
</code_context>
<issue_to_address>
**Partial literals stop completing**
When a supported partial literal is followed by JSON whitespace, `complete_any` passes the whitespace to its literal-prefix checks, which reject the prefix; `parse_json` then raises `MalformedJSON` instead of completing the literal, including when it appears inside a collection or is a special value.
Make literal-prefix checks ignore trailing JSON whitespace while retaining whitespace that belongs to unfinished strings.
</issue_to_address>
### Comment 2
<location path="src/partial_json_parser/core/complete.py" line_range="21" />
<code_context>
try:
- result = complete_any(json_string.strip(), allow, is_top_level)
+ # Trailing whitespace can belong to an unfinished string.
+ result = complete_any(json_string.lstrip(), allow, is_top_level)
if result is False:
raise PartialJSON
</code_context>
<issue_to_address>
**Leading whitespace truncates strings**
When valid leading JSON whitespace precedes an unfinished string, `complete_any` returns an index relative to the `lstrip()` result, but `_fix` applies it to the original `json_string`; the returned head loses part of the string and callers receive a truncated value.
Adjust the returned index for the stripped leading whitespace before `_fix` slices the original string.
</issue_to_address>
### Comment 3
<location path="src/partial_json_parser/core/complete.py" line_range="21" />
<code_context>
try:
- result = complete_any(json_string.strip(), allow, is_top_level)
+ # Trailing whitespace can belong to an unfinished string.
+ result = complete_any(json_string.lstrip(), allow, is_top_level)
if result is False:
raise PartialJSON
</code_context>
<issue_to_address>
**Unfinished strings fail parsing**
When an unfinished string ends with an unescaped tab or newline, or a backslash followed by whitespace, `_fix` preserves the invalid suffix and `complete_str` appends a quote without validating it; `parse_json` passes invalid JSON to its parser, which raises `JSONDecodeError` instead of returning a completed value.
Handle invalid trailing whitespace and dangling escapes in `complete_str` so closing the string produces valid JSON.
</issue_to_address>
### Comment 4
<location path="src/partial_json_parser/core/complete.py" line_range="21" />
<code_context>
try:
- result = complete_any(json_string.strip(), allow, is_top_level)
+ # Trailing whitespace can belong to an unfinished string.
+ result = complete_any(json_string.lstrip(), allow, is_top_level)
if result is False:
raise PartialJSON
</code_context>
<issue_to_address>
**Trailing spaces break partial escapes**
When an unfinished string ends with a truncated `\u`, `\U`, or `\x` escape followed by whitespace, `complete_str` searches for an incomplete `\u`, `\U`, or `\x` escape only within a short suffix. Retained spaces can push the escape outside that search, so it appends a quote after invalid JSON and parsing fails; when the escape is still found, it truncates the escape and the following spaces too.
Handle the incomplete escape separately from subsequent string content so recovery does not discard trailing spaces or emit an invalid escape.
</issue_to_address>Sourcery assessment
Approval pending. 4 findings to address first.
Blocking findings: src/partial_json_parser/core/complete.py:21, src/partial_json_parser/core/complete.py:21, src/partial_json_parser/core/complete.py:21, src/partial_json_parser/core/complete.py:21
| try: | ||
| result = complete_any(json_string.strip(), allow, is_top_level) | ||
| # Trailing whitespace can belong to an unfinished string. | ||
| result = complete_any(json_string.lstrip(), allow, is_top_level) |
There was a problem hiding this comment.
🟠 High · Leading whitespace truncates strings
When valid leading JSON whitespace precedes an unfinished string, complete_any returns an index relative to the lstrip() result, but _fix applies it to the original json_string; the returned head loses part of the string and callers receive a truncated value.
Adjust the returned index for the stripped leading whitespace before _fix slices the original string.
Prompt for AI agents
In `src/partial_json_parser/core/complete.py` at line 21:
**Leading whitespace truncates strings**
When valid leading JSON whitespace precedes an unfinished string, `complete_any` returns an index relative to the `lstrip()` result, but `_fix` applies it to the original `json_string`; the returned head loses part of the string and callers receive a truncated value.
Adjust the returned index for the stripped leading whitespace before `_fix` slices the original string.| try: | ||
| result = complete_any(json_string.strip(), allow, is_top_level) | ||
| # Trailing whitespace can belong to an unfinished string. | ||
| result = complete_any(json_string.lstrip(), allow, is_top_level) |
There was a problem hiding this comment.
🟡 Medium · Unfinished strings fail parsing
When an unfinished string ends with an unescaped tab or newline, or a backslash followed by whitespace, _fix preserves the invalid suffix and complete_str appends a quote without validating it; parse_json passes invalid JSON to its parser, which raises JSONDecodeError instead of returning a completed value.
Handle invalid trailing whitespace and dangling escapes in complete_str so closing the string produces valid JSON.
Prompt for AI agents
In `src/partial_json_parser/core/complete.py` at line 21:
**Unfinished strings fail parsing**
When an unfinished string ends with an unescaped tab or newline, or a backslash followed by whitespace, `_fix` preserves the invalid suffix and `complete_str` appends a quote without validating it; `parse_json` passes invalid JSON to its parser, which raises `JSONDecodeError` instead of returning a completed value.
Handle invalid trailing whitespace and dangling escapes in `complete_str` so closing the string produces valid JSON.| try: | ||
| result = complete_any(json_string.strip(), allow, is_top_level) | ||
| # Trailing whitespace can belong to an unfinished string. | ||
| result = complete_any(json_string.lstrip(), allow, is_top_level) |
There was a problem hiding this comment.
🟡 Medium · Trailing spaces break partial escapes
When an unfinished string ends with a truncated \u, \U, or \x escape followed by whitespace, complete_str searches for an incomplete \u, \U, or \x escape only within a short suffix. Retained spaces can push the escape outside that search, so it appends a quote after invalid JSON and parsing fails; when the escape is still found, it truncates the escape and the following spaces too.
Handle the incomplete escape separately from subsequent string content so recovery does not discard trailing spaces or emit an invalid escape.
Prompt for AI agents
In `src/partial_json_parser/core/complete.py` at line 21:
**Trailing spaces break partial escapes**
When an unfinished string ends with a truncated `\u`, `\U`, or `\x` escape followed by whitespace, `complete_str` searches for an incomplete `\u`, `\U`, or `\x` escape only within a short suffix. Retained spaces can push the escape outside that search, so it appends a quote after invalid JSON and parsing fails; when the escape is still found, it truncates the escape and the following spaces too.
Handle the incomplete escape separately from subsequent string content so recovery does not discard trailing spaces or emit an invalid escape.|
I compared these findings against upstream The follow-up restores upstream recovery for partial literals, trailing control whitespace, and incomplete escapes, while retaining valid trailing string whitespace. It also preserves the existing numeric The leading-whitespace offset issue predates this PR: upstream also truncates unfinished strings with a leading prefix. Open PR #22 already addresses that issue, and this PR's description notes the overlap. I have kept that correction separate to avoid duplicating the existing work. Local verification on Windows / Python 3.12.10: 148 tests pass, including the property and performance test functions. All 94 compatibility cases also pass against the original upstream implementation. Black, isort, and full-source Pyright checks pass. |
An unfinished JSON string loses trailing spaces before completion. For example,
loads('\"hello ')returns'hello'instead of'hello '. The same loss affects strings inside arrays and objects, on both completion paths.Use
lstrip()rather thanstrip()before completion. Add 28 regression/control cases for top-level and nested strings, Unicode whitespace, completed values, and partial-string restrictions. The cases live intests/test_examples.py, which CI runs.Validation on Windows, Python 3.12.10:
e0aa5e2; all 28 pass with this change.src/using the project virtual environment and theplaygroundextra.Related: #22 fixes a separate leading-whitespace offset error in the same function. Its current patch still calls
strip(). This change addresses trailing string content; the patches may need coordination depending on merge order.AI assistance: OpenAI Codex helped investigate, implement, and test this change. The reproduction and verification logs are available for review.