[Birfy]

What this fixes

Fixes #13.

jitagent compile is the first command anyone runs, and the file it takes is JSON you edit by hand. A wrong path, a stray comma, a top-level [], a missing key, or a file saved as UTF-16 each ended in a Python traceback and exit 1:

$ jitagent compile /tmp/nope.json
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/nope.json'

$ jitagent compile /tmp/meta2.json        # the file contains "{"
json.decoder.JSONDecodeError: Expecting property name enclosed in double quotes: line 1 column 2 (char 1)

$ jitagent compile /tmp/meta3.json        # the file contains "[]"
TypeError: list indices must be integers or slices, not str

$ jitagent compile /tmp/meta1.json        # {"requirement": "x"}
KeyError: 'examples'

$ jitagent compile /tmp/utf16.json        # saved with encoding="utf-16"
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xff in position 0: invalid start byte

The traceback reads as "this tool is broken" rather than "my file is wrong", and exit 1 already means something else in this command: the compiled function failed verification. Failing to start is exit 2, which compile already uses when the model is unreachable.

The change

_load_requirement() (one new helper in src/jitagent/cli.py) reads the file once and either returns (meta, examples) or raises BadRequirement carrying a single-line reason. cmd_compile prints that line to stderr and returns 2. Nothing is written to the registry on any of these paths. The decode step is covered separately from the parse step: a UTF-16 save is not JSON and not UTF-8, and "not UTF-8" is the more useful thing to say.

After:

« cannot read '/tmp/nope.json': No such file or directory
« cannot read '/tmp/utf16.json': not UTF-8 text ('utf-8' codec can't decode byte 0xff in position 0: invalid start byte)
« '/tmp/meta2.json' is not valid JSON: Expecting property name enclosed in double quotes: line 1 column 2 (char 1)
« '/tmp/meta3.json' must hold a JSON object with "requirement" and "examples", not an array
« '/tmp/meta1.json' needs "examples" to be a list of seeds
« '/tmp/meta4.json': example 0 needs both "input" and "output" (missing 'input')

(« marks stderr; each is exactly one line, exit 2, no traceback.)

verify has the same shape — Case.load on a bad directory raises FileNotFoundError — and is deliberately not touched here. Same for call, whose input-error path belongs to #6.

Tests

Written first and red on 34b3d2f:

• 8 new tests in tests/test_cli.py, 7 of which fail on 34b3d2f: missing path, non-UTF-8 file, invalid JSON, non-object top level, missing examples, missing requirement, seed without input (asserts the index). Each asserts exit 2, the path in the message, no Traceback, and — for the missing-path case — exactly one line of stderr.
• 1 control test that must pass on both trees: a valid requirement file still cache-hits (L1 hit: rank@v1, exit 0). The guard must not turn a good file into an error.

Evidence on dca2e40 (the pushed head):

pytest -q                                        110 passed   (102 on 34b3d2f, +8)
pytest -q against 34b3d2f with only the new tests 7 failed, 1 passed
python -m jitagent.cli selftest                  6/6 corpus cases behaved as expected
python tools/audit_traps.py                      18/18 traps exercised

The commands above were also run by hand against this head: exit 2 with the messages shown, and compile /tmp/req.json against a seeded registry still hits the cache with no model call.

Risk and rollback

• One command path, one new helper, no new dependency, no registry write on failure.
• Deliberate behaviour change: these inputs move from exit 1 + traceback to exit 2 + one line. That follows the exit-code split the command already uses.
• Rollback: revert the two commits (c3d3c57, dca2e40); the branch is two commits on top of 34b3d2f.

Not in this PR

• verify's identical traceback on a bad case directory (recorded in jitagent compile answers a bad requirement file with a traceback, not an explanation #13 as a non-goal).
• "examples": [] is still accepted by the loader: zero seeds cannot be verified, but that failure belongs to synthesis, and changing it here would widen the PR past its issue.
• The new tests assert substrings, so a BadRequirement subclassed from KeyError (which would wrap the printed line in quotes) could still satisfy them. Reviewer verified this by mutation. Closing it means whole-line assertions; deferred to the next run rather than pushing a third commit after review.
• A requirement file with a non-string "name" (e.g. "name": 5) is still accepted by the loader and stored as that value, which is the --name stores a name no command can reach: compile --name 'a@b' renames the function to an unreachable one #11 class of unreachable name arriving through the file. Pre-existing on 34b3d2f, and outside jitagent compile answers a bad requirement file with a traceback, not an explanation #13's acceptance criteria, so recorded rather than fixed here.
• Deeply nested JSON still escapes: json.loads raises RecursionError (not ValueError) on ~100k nested brackets, so a file like '[' * 100000 still tracebacks, exit 1. Found independently by Test and Security; not fixed here because it is an abuse input rather than a hand-editing mistake, and because the head is frozen for review. One except (ValueError, RecursionError) would close it. Until that follow-up lands, read the CHANGELOG bullet as "invalid JSON within the parser's nesting limit" — that is the precise scope of this change.
• Unbounded read on a non-regular file: a FIFO blocks forever and /dev/zero ends in MemoryError. Both predate this PR (read_text() on that line is unchanged), and neither is a regression.
• No overlap with Fix the credential gate: match key shapes and literals, not credential words #7, Fix jitagent call for inline JSON longer than NAME_MAX #8, Honour the @version pin in jitagent get and inspect #10 or Refuse a name that no reference can reach (compile --name a@b) #12: those touch the credential gate, call's payload, version pinning and --name; this touches the requirement-file loader. The only shared file region is the [Unreleased] section of CHANGELOG.md.

_{Fledge · jitagent · Lead · implemented}

[Birfy]

Posted by jitagent Lead on jitagent Reviewer's behalf.

Review: compile explains an unusable requirement file in one line

Head dca2e40cc015678afb428ad601da2306f253c1f2 (2 commits on 34b3d2fd5d54b3c0fa178076fe86e77518a7b3b6), 3 files, +149/−2. Score: 90/100.

The change lands where it should: _load_requirement() in cli.py turns every "wrong file" shape into one stderr line and exit 2 before any registry call or write; cmd_compile only wraps it. I ran it rather than reading it.

Verified by running

• pytest -q: 110 passed at this head; 102 passed at 34b3d2f — exactly +8, matching the body.
• Reverting only src/jitagent/cli.py to base: 7 of the 8 new tests fail; the valid-file control still passes. The new coverage is real.
• The commit-2 case, a UTF-16 requirement file: exit 2, one stderr line, no traceback, registry untouched. Removing the except UnicodeDecodeError branch fails exactly that new test and nothing else.
• Registry tree byte-identical before/after for missing file, UTF-16, invalid JSON, missing examples, top-level array — each exit 2, one line.
• Shapes beyond the tests: empty file, top-level 5/"x"/null/true, blank/whitespace requirement, non-string requirement, examples: null/string, seeds as list/string/null/int, missing input, directory path, permission denied, Latin-1 binary, UTF-8 BOM — all one line, exit 2, no write. BOM reports "not valid JSON: Unexpected UTF-8 BOM", acceptable wording.
• jitagent selftest 6/6; tools/audit_traps.py 18/18.

Findings — none blocking

1. The tests still cannot tell a clean line from a repr wrapper (recorded, deferred). Mutating BadRequirement(RuntimeError) → BadRequirement(KeyError) in a scratch copy leaves all 15 CLI tests passing: print(e) quotes the whole line, and every substring assertion still matches. Same class as #10/#12. I agree with recording it rather than rewriting seven assertions after review, but it means the suite pins the text, not the rendering. Whole-line assertions are the fix when it is taken up.

2. read_text() has no explicit encoding, so "not UTF-8" is a locale-dependent claim — new in this delta. Path.read_text() without encoding= uses the locale default; on Windows that is a cp1252-family codec under which a UTF-16 file decodes (mojibake), so the user gets is not valid JSON: Expecting value: line 1 column 1 instead of "not UTF-8 text", and the new test fails there. Verified by decoding the same bytes as cp1252. CI is Ubuntu-only, so nothing catches it. p.read_text(encoding="utf-8") makes the branch deterministic and the message true everywhere.

3. "name" is the one field the loader reads and does not check (pre-existing, deferred). On this head, a file with "name": 5 stores an int name: jitagent list shows 5, and get("5") and get("rank") both exit 1 — unreachable, the #11 class through the file. On #12's tree check_name(5) raises AttributeError: 'int' object has no attribute 'strip', so after #12 merges the same file tracebacks again. One isinstance(meta["name"], str) here (or a non-str-safe check_name) closes both. Agreed it is follow-up scope, but the two PRs intersect exactly here and it should not be lost.

4. examples: [] stays accepted — right call, and worth knowing where it lands. With gen_tests=8, propose_tests fills the seeds, so empty is a legitimate "let the model write the cases". On a cache miss with --via api and no credentials it exits 2 with the (multi-line, pre-existing) credentials message; with the default --via cli and no claude binary the synthesis path raises FileNotFoundError as a traceback — pre-existing, unrelated to this loader, but it is the path an accepted empty file takes. One test pinning that an empty file reaches synthesis would make the decision durable.

5. Deferrals I checked and agree with. Deeply nested JSON ('[' * 100000) escapes as RecursionError and still tracebacks; that is abuse input, not a hand-edit mistake, and the body records it. verify's identical traceback is a recorded non-goal of #13.

Test-coverage nits (non-blocking)

• Only the missing-file test asserts a single line; the other seven assert "no Traceback" plus a substring, so a future two-line message (an added hint line) still passes six of them.
• The sharpened seed message — "needs to be an object with input and output" — has no test: the only seed test takes the KeyError branch (missing output). A seed that is a list/string/null takes the TypeError branch, which I exercised by hand.
• The blank-requirement branch (.strip()) is untested; the requirement test covers only the key being absent.
• line 1 column 2 pins CPython's JSON wording; fine across 3.11–3.13, as accepted.

On the specific questions

(a) The set is right. Six file-shape failures plus the decode failure cover every hand-edit mistake I could construct (above), and each message names the path and the thing to change — testable claims, not prose. The only gap left in the "bad file" class is the adversarial RecursionError, correctly deferred.

(b) examples: [] accepted on purpose is correct — see finding 4; document or pin it, do not reject it.

(c) The 1/2 split is coherent: 2 = the command could not start (bad file, no backend/credentials), 1 = it ran and the verdict failed. It matches the pre-existing NoCredentials return, so scripts can treat 2 as "fix your setup, nothing happened". It is undocumented — the CHANGELOG says exit 2 but never says 1. One clause ("2 = could not start, 1 = ran and failed") turns it into a contract instead of a coincidence.

(d) No — the repr hole survives, verified by mutation (finding 1). Everything else in the new tests fails when the corresponding code is reverted: checked for the UTF-8 branch, and wholesale by restoring base cli.py (7/8 red, control green).

Not run

• Python 3.11 and 3.13: only CPython 3.12.16 is available here.
• Windows: no runner and not in CI; finding 2 is the consequence.
• CI check-runs are green on this head (3.11, 3.12, 3.13, package). Note the commit's committer stamp is ~28 minutes ahead of the CI run's clock, so do not read the timestamps as order.

Verdict: 90/100. The bug this PR exists to fix is fixed, with a regression test that fails on the old code. The remaining gaps are the deferred assertion hole, one encoding word, and the deferred name check — none blocks the stated issue.

_{Fledge · jitagent · Reviewer · reviewed}

[Birfy]

Fledge automation · jitagent · job job_MF7p8bc6LI7tjA (schedule, iterate)

Automated run: Lead claimed + implemented · Test tested · Reviewer reviewed.

Fledge never merges; a human must review and merge.

• Issue: jitagent compile answers a bad requirement file with a traceback, not an explanation #13
• Team console: https://fledge.hugoxk.online/repos/repo_al_v9DflQ4qkmoDD/team

[Birfy]

Re-review at dca2e40c — 90/100 (head unchanged since 5736643339)

Head dca2e40cc015678afb428ad601da2306f253c1f2 (2 commits, 3 files, +149/−2). The tree I reviewed is byte-identical to GitHub's tarball at this SHA, and there are no commits since my review 5736643339, so that score stands; I re-read the diff and re-ran the suite here rather than restating yesterday's numbers.

• pytest -q in that tree: 110 passed (36.7 s).
• CI on this SHA: 4/4 success — package, test (3.11), test (3.12), test (3.13) (run 41444879669; all four jobs checked).
• The shape is right: _load_requirement() turns every unusable-file case into one stderr line and exit 2 before any registry call or write, and cmd_compile only wraps it.

Open, non-blocking (unchanged from 5736643339): the repr-based assertion hole I demonstrated by mutation; read_text() has no explicit encoding=, so the "not UTF-8 text" wording is locale-dependent; the exit-code contract is documented for 2 but not for 1.

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

_{Fledge · jitagent · Reviewer · reviewed}

[Birfy]

Re-review at dca2e40c — 90/100 (head unchanged since 5761485732)

Head dca2e40cc015678afb428ad601da2306f253c1f2; no commits since that review, so the 90 stands. Re-verified today: pytest -q → 110 passed, CI 4/4 (run 41444879669), mergeable_state: clean; and by hand at the CLI — an invalid-JSON file, a top-level array and a UTF-16 file each give exactly one stderr line, exit 2, no Traceback, and nothing written to the home.

Open, non-blocking (unchanged): the repr-based assertion hole I demonstrated by mutation; read_text() has no explicit encoding=, so the "not UTF-8 text" wording is locale-dependent; the exit-code contract is documented for 2 but not for 1.

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

_{Fledge · jitagent · Reviewer · reviewed}