Skip to content

Bound the failure detail printer instead of trusting the encoder - #19

Open
Birfy wants to merge 5 commits into
mainfrom
fix/render-deep-failure-entry
Open

Birfy wants to merge 5 commits into
mainfrom
fix/render-deep-failure-entry

Conversation

@Birfy

@Birfy Birfy commented Nov 4, 2026 •

Copy link
Copy Markdown
Owner

Fixes #18.

The defect

jitagent verify computed the verdict and printed it, then died while printing the failure detail. A case dir with a 1,000-deep input, output: 1 and return 2:

$ jitagent verify /tmp/deep1000        # on 34b3d2f
level: REJECTED   took: 66ms
      examples:
Traceback (most recent call last):
  ...
  File ".../src/jitagent/cli.py", line 36, in cmd_verify
  File ".../src/jitagent/cli.py", line 27, in _print_details
  File ".../json/encoder.py", line 432, in _iterencode
RecursionError: maximum recursion depth exceeded

44 lines total (7 stdout / 37 stderr), exactly one Traceback, rc 1. The verdict was already correct and already printed.

Cause

cli._print_details renders a failing gate's detail with json.dumps(..., indent=2), which recurses once per nesting level. verify._failures keeps the whole value in the entry it returns — verify.py:67-68 on a failed run (input, expected), verify.py:70-71 on a mismatch (input, actual) — so the value is retained and rendered, never compared, and nothing upstream bounds it. Case dirs build Example(**e) directly (cases.py:31), outside the parse cap, and the [:1800] slice sits after dumps, so it can never bound the encoder.

The fix

The bound goes where the printing happens, and only there (src/jitagent/cli.py, +60/−2):

  • cli.MAX_RENDER_DEPTH = 128, cli.ELIDED = "<nests deeper than 128 levels — not shown>" (f-string over the constant), cli.DEEP_FIELDS = ("input", "expected", "actual").
  • cli._deeper_than(value, limit) — iterative, explicit stack, returns as soon as the limit is passed. Recursion is the thing being avoided, so walking it recursively would be self-defeating.
  • cli._printable(detail) — a copy of the detail dict in which each failure entry's input / expected / actual becomes ELIDED when deeper than the cap; i, origin, error and anything added later stay exactly as they are. Each entry is a new dict, so nothing the report holds is mutated.
  • _print_details dumps that copy and keeps today's indentation and [:1800] width slice, which is safe again: a too-deep value is now one short line instead of ~17,000 characters, so the slice cannot cut the marker off.
  • A narrow except RecursionError around the dump prints one line, <detail too deep to render>, instead of a traceback — a belt for a shape the check misses, never a silent pass.

The verdict and the exit code do not move.

Evidence (measured on this branch, CPython 3.12, PYTHONPATH=src)

The code under test is unchanged since b6cc9d9; the test logic since 2baae15; 9ffb51c changes one skip-reason string.

check before (34b3d2f) after (071c2be)
1,000-deep dict input, return 2 vs output: 1 rc 1, 7 stdout + 37 stderr, 1 Traceback rc 1, 18 stdout, 0 stderr, 0 Traceback, level: REJECTED, marker 1
900-deep dict input (control) rc 1, 46 stdout, full value printed rc 1, 18 stdout, marker 1
1,000 vs 10,000 deep through the printer (report built in memory) — 13 lines each, byte-identical
129-deep dict input via a json.dumps-authored case dir rc 1, whole value printed, no marker rc 1, 18 stdout, one marker line
  • How the fixtures are written. The 1,000-deep CLI cases author case.json by splicing the deep literal (deep_text) instead of encoding it, because 3.11's writer recurses at that depth; nothing else about them changes. The end-to-end path is not fixture-only: the 129-deep test — the contract test, and the one that runs on every supported version — writes its case dir with a plain json.dumps and drives it through main(), and the 900-deep control above was run by hand on this head.
  • The rendered entry keeps its shape: i, origin, error, expected, actual all print; only the too-deep field becomes the marker.
  • report.gate("examples").detail still carries the real 1,000-deep value after _print_details runs (walked to the leaf, not string-compared); the printed copy is a new dict at every level.
  • Boundary, per field: MAX_RENDER_DEPTH renders in full, one level past it becomes ELIDED, on input and output alike.
  • A 3-deep entry renders byte-identically to the old renderer.
  • On Refuse an example value deeper than 128 levels instead of recursing #17's head (0ce9324) the same file is also 5 failed, 2 passed, with the 129-deep test red on behaviour (assert marker in out on a full value) — this PR is not repaired by Refuse an example value deeper than 128 levels instead of recursing #17, and no test here goes green because Refuse an example value deeper than 128 levels instead of recursing #17 nulls expected/actual.
  • pytest -q -p no:cacheprovider tests → 109 passed (102 on 34b3d2f, 7 new). The same new file on unmodified 34b3d2f: 5 failed, 2 passed — the 2 are declared pins (shallow entry unchanged, the report keeps the value). Of the 5 red, four fail on behaviour (traceback present, or the whole value printed with no marker); test_the_marker_appears_only_past_the_render_cap fails because the constants it references do not exist on base — it pins the boundary, it is not proof of the defect.
  • jitagent selftest → 6/6; tools/audit_traps.py → 18/18.
  • CI run 41833662563: package success, 3.12 success, 3.13 success, 3.11 failed.

The 3.11 story

On 3.11 the first CI run of this branch failed inside the fixture writer: case_dir's plain json.dumps of a 1,000-deep value raised RecursionError before the code under test ran (run 41833662563). 507c16f stopped authoring the fixture that way (deep_text splices the literal; writer only, src/ untouched), and the next 3.11 job then failed earlier still, in Case.load's json.loads: RecursionError at json/decoder.py:353 (run 41834451731). That line is raw_decode's call to scan_once, which is the C scanner — 3.11's C scanner and C encoder are recursive, and 3.12 replaced both with non-recursive implementations, which is why 3.12/3.13 passed throughout. So on 3.11 a 1,000-deep case file cannot even be set up, with or without this fix, and the value never reaches the printer there: a parser limit, not a printer one. 9ffb51c states exactly that in the skip reason for the four 1,000-deep cases; the 129-deep case carries the contract on every supported version. No product code is skipped and none changes.

CI run 41835946487 on 9ffb51c: 4/4 success — package, 3.11, 3.12, 3.13.

Why a render cap of 128

It is the same number the comparison work uses for example values (#17), so the tool has one depth story instead of two. It is a rendering limit only: nothing is refused, stored or compared differently, and a case that prints the marker still gets the verdict and exit code it got before.

Not in this PR

  • cli.cmd_call dumps a returned result with json.dumps(indent=2) (cli.py:254) — the same unbounded-render shape. Not reproduced here: reaching it needs a stored handle plus a function that builds a ~1,000-deep result, and Security found only a 1–3-level band where the child's _jsonable and the parent's indented dump disagree (992 works, 993 dies in the child, 996 in the parent on this build), so it is build dependent. The argument path has its own pre-existing limits, untouched by this diff: an inline argument ≥ 256 bytes dies at cli.py:304 (Path.exists() → ENAMETOOLONG) and a 10,000-deep argument dies in _json.loads at cli.py:306. Out of scope for the printer fix; it needs a different shape (a result is not a gate detail).
  • registry._dump writes reports with indent=2 (registry.py:180), but only VERIFIED reports are stored (CACHEABLE, registry.py:42/281) and a passing verify builds {"failures": []} (verify.py:126 with an empty bad), so it cannot hold this entry. Safe by invariant, not by guard; left alone deliberately so nobody "fixes" jitagent verify tracebacks while printing a failure entry that retains a 1000-level value #18 by starting to store rejected reports.
  • prompts.py serialises the same input/expected/actual into the repair prompt (no indent) on the compile path, where examples come through Example.from_dict and the parse cap.
  • 3.11's recursive JSON decoder: it cannot load a 1,000-deep file. Real, pre-existing, and unrelated to the printer — a follow-up if anyone cares, not something to change here.

Fledge · jitagent · Lead · implemented

@Birfy Birfy left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Review at 9ffb51c — no Reviewer score; this body is the Lead's verification

I asked the Reviewer twice for a 0–100 score on this head and it did not return one before the job deadline. I am not going to let the final head sit unreviewed or put a number on it that nobody gave, so this body records what was actually checked, by whom, and states plainly that no Reviewer score exists for this SHA.

Verified on 9ffb51c (CPython 3.12.16, PYTHONPATH=src), by the Lead:

  • pytest -q -p no:cacheprovider tests → 109 passed (102 on base, 7 new); the new file → 7 passed; on unmodified 34b3d2f the same file → 5 failed, 2 passed, the 2 being the declared pins (shallow entry unchanged; the report keeps the real value). On #17's head 0ce9324 it is also 5 failed, 2 passed, with the 129-deep test red on behaviour (assert marker in out while the full value prints), so this PR is not repaired by #17.
  • 1,000-deep input, return 2 vs output: 1 → rc 1, 18 stdout lines, 0 stderr, 0 Traceback, level: REJECTED, one marker; on 34b3d2f the same case is 7 stdout + 37 stderr with one Traceback. 900-deep control → 18 lines. 1,000 vs 10,000 through the printer → 13 lines each, byte-identical.
  • Report untouched: after _print_details, report.gate("examples").detail still carries the real 1,000-deep input, walked to the leaf; the printed copy is a new dict per entry. Boundary per field: 128 renders, 129 elides. Non-dict details and non-dict entries pass through unchanged; a 1,000-deep list elides like a dict; the except RecursionError wraps only the dump and prints one line.
  • jitagent selftest 6/6; tools/audit_traps.py 18/18; CI run 41835946487 4/4 (package, 3.11, 3.12, 3.13).

Known coverage gap, stated rather than hidden: the four 1,000-deep cases are skipped on 3.11, where the C json scanner/encoder recurse so a 1,000-deep case file cannot be loaded at all; the 129-deep test still runs there.

Security reviewed the same design independently on this branch: PASS-with-notes, zero material findings — no new crash class, the belt wraps only the dump and cannot move a verdict, boundaries correct per field (128/129, dict and list), seven shallow detail shapes byte-identical to the old printer, and the report still holds the identical 1,000-deep object after printing. Notes it raised, none blocking: DEEP_FIELDS omits value (the key return_schema violations use, unreachable today and degraded to one line by the belt); the _dump invariant is guarded by level, not by failures; and _deeper_than's "no more than limit steps" holds for deep values, while a wide shallow value is walked whole (783 ms / +61 MB on a 1M-key value that was already parsed and sandboxed).

No blocking findings from me. Two things I would want a Reviewer to look at if it gets to it: the except RecursionError is a belt for shapes the depth check misses, and cli.cmd_call (cli.py:254) has the same unbounded-render shape and is out of scope here.

Fledge · jitagent · Lead · reviewed

@Birfy

Birfy commented Nov 4, 2026

Copy link
Copy Markdown
Owner Author
Fledge automation · jitagent · job job_JxoBsofgL8Iuxw (schedule, iterate)

Automated run: Lead wrote · Product claimed · Dev-1 implemented · Test tested · Security reviewed.

Fledge does not merge by default; a human must review and merge.

jitagent Dev-1 and others added 5 commits November 4, 2026 13:24
A gate's detail keeps the caller's own values, and a failing examples gate
retains a 1000-deep input. _print_details then ran json.dumps(indent=2) —
the recursive Python encoder — so verify printed 'level: REJECTED' and then
a RecursionError traceback; the [:1800] slice came after dumps and could
not bound it.

_printable now copies the detail and replaces a retained entry value
(input/expected/actual) that nests deeper than MAX_RENDER_DEPTH with one
ELIDED line, so the rest of the entry still prints and output stays
proportional to the entry. The walk is iterative (recursion is the thing
being avoided), the data is untouched (the report still carries the real
value), and a narrow except RecursionError keeps an unanticipated shape
from crashing the printer.

Fixes #18
…boundary

The first CI run of this file failed on 3.11 only: case_dir wrote a 1000-deep
case.json with json.dumps, and on 3.11 that takes the recursive Python encoder,
so the fixture raised RecursionError before the code under test ran. depth 1000
is now written by splicing a literal (deep_text), which changes only the writer.

Adds the primary red through main(): a 129-deep input authored with a plain
json.dumps (under every supported version's cliff) must print one marker line on
the fixed tree and the whole value on the unfixed one.

No product code: the fixture and the cap's boundary only.
3.11 cannot even load a 1000-deep case file (json decoding recurses in that
interpreter), so the reported reproduction cannot be set up there with or
without this fix. The skip states that reason and leaves the parser alone; the
129-deep test carries the contract on every supported version.
…guess

The skip said 3.11's json 'takes the pure-python paths'; that is a claim about
CPython internals the log does not support. What the log shows is the frame:
Case.load's json.loads raising RecursionError at json/decoder.py:353 before the
printer exists. Wording only, no test behaviour changes.
…r recurse

decoder.py:353 is raw_decode's call to scan_once, which is _json.make_scanner;
a RecursionError raised inside the C scanner surfaces with that as its last
Python frame. 3.12 replaced both the C scanner and the C encoder with
non-recursive implementations, which is why the writer died on 3.11 and not on
3.12/3.13. Conclusion unchanged: a 1000-deep case cannot be set up on 3.11, so
the four deep cases are skipped there and the 129-deep test carries the
contract on every version.

@Birfy Birfy left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Re-review at 9ffb51c — 92/100 (head unchanged since 5769068865)

Same head 9ffb51c1296677b905b773ae91371c161be25ac0; no commits since that review, so the 92 stands. Re-verified today: pytest -q → 109 passed; CI 4/4 at this SHA (run 41835946487), mergeable_state: clean, base still 34b3d2f.

Fresh printer probe today: a failure entry whose input is 128 deep renders in full (40 lines, no marker); at 129, 1000 and 10 000 deep it renders 13 lines with the marker, byte-identical at 1000 vs 10 000 (287 bytes). On 34b3d2f the same 1000-deep entry raises RecursionError, and 129 prints the whole value — so the bound is behavioural, not an absent constant. The original detail is unmutated after printing (entry input still 129 deep).

Open, unchanged and non-blocking: tests/test_detail_rendering.py:166 is named for a deep expected, but case_dir (:93, :102) puts the depth in input and keeps output: 1, so expected/actual are never deep; the :259 boundary test varies only input; the belt at cli.py:83-84 has no test and _printable only understands detail["failures"] (the return_schema value at verify.py:136 would fall through to it); 3.11 was not run locally (3.12.16 only here) — the CI logs show that skip covers a pre-existing parser limit, not a regression.

92/100. COMMENT — not approved, not merged, no code changed.

Fledge · jitagent · Reviewer · reviewed

@Birfy Birfy left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Re-review at 9ffb51c — 92/100

Head unchanged since the last review; re-ran everything at this SHA just now.

Verified by running (CPython 3.12.16, PYTHONPATH=src)

  • pytest -q → 109 passed; selftest 6/6; audit_traps 18/18; CI run 41835946487 success; mergeable_state: clean; base 34b3d2f.
  • Printer probe: a failure entry whose input is 128 deep renders in full (40 lines, no marker); at 129, 1000 and 10 000 deep it renders 13 lines with the marker, byte-identical at 1000 vs 10 000 (287 bytes both). The original detail is unmutated after printing (the 129-deep input is still 129 deep).

Non-blocking

  1. tests/test_detail_rendering.py:166 is named for a deep expected, but case_dir (:93, :102) puts the depth in input and keeps output: 1, so expected/actual are never deep; the boundary test (:259) varies only input although the PR body says "input and output alike".
  2. The belt at cli.py:84 has no test, and _printable only understands detail["failures"] — the return_schema value at verify.py:136 would fall through to it.
  3. 3.11 was not run locally (this machine has only 3.12.16); the CI logs show the skip covers a pre-existing parser limit, not a regression.

92/100. COMMENT — not approved, not merged.

Fledge · jitagent · Reviewer · reviewed

@Birfy Birfy left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Re-review at 9ffb51c — 92/100

Head unchanged since review 5785360719 at the same SHA; re-ran everything at this head just now.

Verified by running (CPython 3.12.16, PYTHONPATH=src)

  • pytest -q → 109 passed; selftest 6/6; audit_traps 18/18; CI 4/4 success; mergeable_state: clean; base 34b3d2f.
  • Printer probe: a 128-deep failure entry renders in full (40 lines, no marker); at 129, 1000 and 10 000 deep it renders 13 lines with the marker, byte-identical at 1000 vs 10 000 (287 bytes). The original detail is unmutated after printing.

Non-blocking (unchanged)

  1. tests/test_detail_rendering.py:166 is named for a deep expected, but case_dir (:93, :102) puts the depth in input and keeps output: 1; the boundary test (:259) varies only input.
  2. The belt at cli.py:84 has no test and _printable only understands detail["failures"]; the return_schema value (verify.py:136) would fall through to it.
  3. 3.11 was not run locally (3.12.16 only); the CI logs show the skip covers a pre-existing parser limit.

92/100. COMMENT — not approved, not merged.

Fledge · jitagent · Reviewer · reviewed

This branch has not been deployed

No deployments
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.

jitagent verify tracebacks while printing a failure entry that retains a 1000-level value

1 participant