[antonwolfy]

Summary

parse_json(answer, contains=True) (#619) capped the number of raw_decode tries at CONTAINS_JSON_MAX_STARTS (1000), not the characters each try scans. On main, parse_json('[' * 900 + '1,' * 100000, contains=True) returns (False, None) after about 2.3 s on Linux, because each of the 900 tries parses the whole tail before it fails at the end of the text.

The issue offers two fixes; neither is taken. Stopping after the first try that consumed to the end would drop '[ {"a": 1}, [1,', whose first [ fails at the end while the object after it is valid JSON (test_parse_json_contains_finds_a_value_after_a_start_too_deep_to_decode pins that a later start is still found). A window per try would refuse a valid long value. Instead the total work is bounded: each failed try adds the characters it scanned (the JSONDecodeError.pos minus its start) to a running total, and the scan reads the text as not JSON once the total passes CONTAINS_JSON_MAX_SCAN (1,000,000), as it does at the start cap.

Closes #619

Changes

• dokimi/eval/reference_checks.py: new constant CONTAINS_JSON_MAX_SCAN; the contains loop in parse_json sums the scanned characters of failed tries and stops once they reach the constant (the try that reaches it may cross it). A try that raises ValueError (an overlong integer) or RecursionError (the depth limit) carries no position and is charged the rest of the text, and an unterminated string, which reports its opening quote, is charged to the end of the text. A scan that gives up reads as not JSON, and the changelog fragment says so, with the input '[' * 6 + '1,' * 100000 + ' {"a": 1}', which finds the object on main and fails now. CONTAINS_JSON_MAX_STARTS and its behaviour are unchanged. The docstring and the constants' comment name the new cap.
• docs/answer-formats.md: the json row names the scan budget next to the 1000-bracket limit.
• tests/test_fuzz_targets.py: the issue's repro returns (False, None) with raw_decode called at most 7 times (a spy, no timing); a value after late-failing starts within the budget is still found; failures without a position (overlong integer, depth limit, unterminated string) are charged and bounded the same way.
• tests/test_reference_checks.py: a json check with contains whose scan gives up fails as not JSON (it is not left unscored as a regex check past its limit is), and the same shape within the budget passes.
• Changelog fragment (Fixed).

raw_decode is called from this one site in the package; no other loop decodes from many start offsets.

Risk

A text whose failed tries scan 1,000,000 characters or more in all before the first valid value now reads as not JSON, where it was found (slowly) before; six brackets followed by 200,000 characters and then an object are enough ('[' * 6 + '1,' * 100000 + ' {"a": 1}'). The check fails as not JSON; it is not left unscored. The same holds for a value before a long run of brackets: a try that hits the depth limit carries no position and is charged the rest of the text, so '[' + '{"a": 1},' + '[' * 1_100_000 reads as not JSON (the object is found on main); charging such a try nothing would bring the cost back, since 1000 depth-limit tries that each scan a 1,000,000-character flat prefix took 10 s. A found value of any length when no earlier try failed late, and a value after stray brackets whose failed tries stay under the budget, are unchanged. Open PR #472 also edits eval/reference_checks.py; this change touches only the constants and the contains loop of parse_json, so a conflict is possible only if #472 edits those lines.

Testing

On a Linux review host, at 63b6a04 (on main c1d7f5a):

• make ci-local: 30 passed, 2 failed, 14 skipped (docker and network steps, micromamba, semgrep, the Intel CI matrix, fuzz, the other Pythons). The 2 failures are the gitleaks download (HTTP 504) and the commit scan that needs it. 19959 tests passed, 19 skipped.
• Repro: parse_json('[' + '{"a":1},' + '[' * 1_100_000, contains=True) returns (False, None) at the head (the object is found on main); the commit of this round changes only the changelog fragment, so it has no mutant.
• Repro: on main parse_json('[' * 900 + '1,' * 100000, contains=True) takes 2.25 s and calls raw_decode 901 times; here the new test sees at most 7 calls.
• Mutants: dropping the scan check fails test_parse_json_contains_bounds_the_characters_failed_tries_scan (901 calls); the test also fails on main's code (the constant does not exist there).
• Repro: parse_json('[' * 6 + '1,' * 100000 + ' {"a": 1}', contains=True) returns (False, None) at the head (it finds the object on main); the new test_a_contains_json_scan_that_gives_up_is_failed_not_unscored pins the check result not JSON. Mutants: raising the budget to 10**12 fails that test at its first assertion; a budget of 0 fails it at the within-budget assertion.
• Repro: parse_json('[1' + ' ' * (CONTAINS_JSON_MAX_SCAN - 2) + '2] {"a":1}', contains=True) (the first try fails at position 1,000,000, charging exactly the budget) returns (False, None) at the head and (True, {'a': 1}) with the guard as >; the same text one space shorter finds the object. test_parse_json_contains_stops_when_the_failed_tries_reach_the_budget_exactly pins both. Mutant: the guard back to > fails it at the first assertion.
• Not run: Python 3.11, 3.12 and 3.14, and the Docker and network steps.

Revert

A plain revert restores the old behaviour (the start cap alone). Nothing is stored.

Checklist

• Tests added/updated and pytest passes
• ruff, mypy, and schema checks pass
• Docs updated (or N/A) — see the docs-drift checklist in CONTRIBUTING.md
• Self-reviewed per "Before opening a PR" in AGENTS.md
• No ad-hoc run artifacts committed
• New files carry the SPDX license header (or are exempt in .github/.licenserc.yaml)

[exai-bot]

✅ OpenCodeReview: Review complete: 0 finding(s) across 3 selected item(s).

CI at 63b6a04: 26 passing, 3 skipped.

Review limit reached. This PR has had 6 reviews, so it will only be reviewed again when you ask.
To ask: re-request review from exai-bot in the Reviewers list.

Review details

• Commit reviewed: 63b6a04 against merge base 1ed2c6a
• Coverage: 3 files in 1 groups
• Engine: OpenCodeReview v1.13.5-exai.4.8f73eada, us.openai.gpt-6.1-sol
• Usage: 59k tokens (35k from cache), 17s
• Tool calls: 8 (4 file_read, 1 ci_status, 1 code_search, 1 local_check_results, 1 pr_threads)
• small change set: 3 files

[vlad-perevezentsev]

Review at 475bf87. The fix works for the case it names: '[' * 999 + '1,' * 1_500_000 drops from 44 s to 0.1 s. But three kinds of failed tries are still charged almost nothing, so the same cost remains for other long answers.

Agreed with @exai-bot on all three: the unterminated string, the long integer and the RecursionError path. Measured numbers are in the replies.

What I ran, at 475bf87:

• make ci-local --offline: 1 failed, 20 passed, 19 skipped. The failure is schema_compat: main added fields to novelty.v1.json after this branch's base (#633); this PR does not touch schemas.
• Timings on the head and the base, 999 starts and a 3M-character tail: the PR's case 0.1 s vs 44 s; unterminated string 3.2 s vs 3.2 s; long integer 2.7 s vs 2.7 s; flat string then deep nesting 3.6 s vs 3.6 s.

Checked, no issues

• The new test: it fails on the base (no budget, 900 tries) and its call bound matches the code.
• Docs and changelog: answer-formats.md names the new limit, and the fragment is under Fixed.

[vchamarthi]

Pushed 1 commit to fix/json-contains-scan-cost (475bf87..b99e2d1).

• Docstring says a failed first try counts.
• ValueError, RecursionError and unterminated-string failures are charged len(text) - i.
• Docstring and changelog say the first whole-text parse is not counted, only tries that start at a bracket.
• Test pins that a failed try is charged end - i.

Threads: 3 fixed, 0 answered as not a defect.
Run: the full suite with main merged in, 19145 passed.

Questions for @antonwolfy

1. Should an exhausted scan budget return None, "not scored: ..." as _regex does, or keep reading as not JSON with the changelog saying so? (dokimi/dokimi/eval/reference_checks.py:270)
'[' * 6 + '1,' * 100000 + ' {"a": 1}' finds {"a": 1} on main and reads as not JSON on the head.

• A: Return not scored when the budget or start cap is hit: needs a signal from parse_json to its callers.
• B: Keep not JSON and state in the changelog that such answers now fail.

[antonwolfy]

(commented) Review at 475bf87. The fix works for the case it names: '[' * 999 + '1,' * 1_500_000 drops fr

@vlad-perevezentsev (#707 (review)): The three review threads (long integer, depth limit, unterminated string) are answered in 4565022466, 4565023276 and 4565022850: charged in b99e2d1 and passing at the head. The not-scored question is answered in 4563462339. The schema_compat failure you saw at 475bf87 does not occur at this head: make ci-local here has 0 failed, 32 passed, 13 skipped.

[antonwolfy]

Pushed 1 commit to fix/json-contains-scan-cost (475bf87..b99e2d1).

@vchamarthi (#707 (comment)): Question 1: option B. An exhausted budget stays "not JSON" and the changelog says so, with your example (c6741e2, c0f22ca); the reasoning is in the reply to 4563462339. The three charged failure kinds from b99e2d1 are confirmed in the code and tests at the head.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.72%. Comparing base (c1d7f5a) to head (63b6a04).
⚠️ Report is 10 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #707      +/-   ##
==========================================
- Coverage   95.73%   95.72%   -0.01%     
==========================================
  Files         408      414       +6     
  Lines       54644    55315     +671     
  Branches     9404     9509     +105     
==========================================
+ Hits        52311    52948     +637     
- Misses       1442     1465      +23     
- Partials      891      902      +11     

Files with missing lines	Coverage Δ
dokimi/dokimi/eval/reference_checks.py	95.68% <100.00%> (+0.05%)	⬆️

... and 34 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[napetrov]

Change class: not computed (no receipt for this repo in this checkout)

Verdict: comment only, no approval. One question on the budget design, flagged for a human decision. Head c0f22ca, merge base c8e707c, lane L1.

CI: 0 failing Actions jobs on head; the merge base is behind the base tip, no red to attribute.

Design (human call): how much budget a failed start may take, and whether a value after a long failed start must still be found. Not a one-way door, so no blocking finding. Thread C2 is the probed case; T4 is the 6-bracket case already answered.

Coverage: plain and edge pass on head and base; probe of the 1.1M-bracket input on both trees. Not run: pytest (regex not installed), ruff, mypy, Python 3.11 and 3.14 (only 3.12 here).

[xaleryb]

• Ran: head c0f22ca merged with main 60044a8 on Python 3.13: full suite, ruff, mypy, bandit -ll, docs check --strict (rc 0). Re-ran every input reported earlier in this thread: each one now ends in about 0.01 s.
• parse_json('[' + '1'*5000 + '] ' + 'x'*1_000_000 + ' {"a": 1}', contains=True) -> (False, None); main gives (True, {'a': 1}). The try at index 0 reads about 5000 characters but is charged to the end of the text. napetrov's depth-limit input behaves the same way. The changelog names this charge rule, but the json row in docs/answer-formats.md says the scan stops only after 1,000,000 scanned characters. Expected: the row says what the code does.
• Pushed (c0f22ca..e9e2edf): e9e2edf, a doc-only change. The json row now says that a try failing on an overlong number, the depth limit or an unterminated string counts as scanning to the end of the text. The code is unchanged and still follows option B.
• Left for the author: none.

[xaleryb]

Re-checked on e9e2edf merged with main: every earlier cost input ends in about 0.01 s, and the json row in docs/answer-formats.md now says how a failed try without a position is charged, matching the code and the changelog; the full suite, ruff, mypy, bandit and docs check --strict pass, and CI is green.

[antonwolfy]

(commented) Change class: not computed (no receipt for this repo in this checkout)

@napetrov (#707 (review)): The review body only carries the budget question of thread 4568041537, answered above.

[antonwolfy]

(approved) Re-checked on e9e2edf merged with main: every earlier cost input ends in about 0.01 s, and the json row in docs/answer-formats.md now says how a failed try without a position is charged

@xaleryb (#707 (review)): Approval; it recaps its checks. Nothing to change. The json row in docs/answer-formats.md already says that a depth-limit try counts as scanning to the end of the text.

[antonwolfy]

• Ran: head c0f22ca merged with main 60044a8 on Python 3.13: full suite, ruff, mypy, bandit -ll, docs check --strict (rc 0). Re-ran every input reported earlier in this thread: each one now ends in

@xaleryb (#707 (comment)): The comment recaps its re-run and e9e2edf (the json row now says how a try without a position is charged); I checked the row against parse_json at this head and it matches. The depth-limit input it mentions is the one of thread 4568041537, now named in the changelog fragment (1ed2c6a).

[antonwolfy]

Ready for another look at 63b6a04; CI is green.

Declined, with the evidence in the reply:

• dokimi/dokimi/eval/reference_checks.py:273 (@napetrov) (fixed in part): Restoring the old result is declined: a depth-limit try has no position and can scan a long flat prefix first (1000 such tries took 10 s), so charging it nothing brings the cost back.