sequenceDiagram
  participant Caller
  participant load
  participant StreamingParser
  participant EvalData
  Caller->>load: provide evaluation path
  load->>StreamingParser: select parser by size and extension
  StreamingParser-->>load: yield parsed rows or parsed JSON records
  load->>EvalData: construct adapter-normalized data
  EvalData-->>Caller: return EvalData

_{Comment @coderabbitai help to get the list of available commands.}

I looked at the streaming path and memory claims only.

1. Peak memory still scales with row count

Issue acceptance asks for peak memory bounded by the streaming buffer, not file size. The generators do avoid Path.read_text() for large JSONL/CSV. That removes the full-file string. Peak still scales with row count because the stream is re-materialised:

    rows = list(row_iter)
    ...
    records = dicts_to_records(rows, skipped)

Same pattern for CSV (rows = list(_iter_csv_rows(...))) and for ijson.items → list(...).

Measured on head 1ade634 vs main 9aebe24, separate processes, macOS ru_maxrss in MiB:

String elimination is real on the padded file (~313 MiB tracemalloc, ~228 MiB RSS). On the compact dump the peak barely moves and stays multi-GiB for a 70 MiB file. So larger-than-RAM audit is still not met for object-heavy score dumps.

Possible direction: convert each row to a Record inside the iteration (or in small batches) so the full list[dict] never coexists with the record list. Longer term, that needs detect_line_adapter / dicts_to_records to accept a one-row lookahead instead of a full list. Until then it may be clearer to state the win as "no full-file string for large JSONL/CSV" rather than "peak bounded by the streaming buffer".

2. Optional ijson changes large-JSON success, not only memory

ijson is not in pyproject.toml dependencies or optional-deps. For a large .json top-level array, the path forks on whether it happens to be installed.

Minimal repro (force the large path with _STREAM_THRESHOLD = 0):

# f2_min.json = [{"id":"q1","model":"A","score":1.0},{"id":"q1","model":"B","score":0.0}]
from unittest import mock
from evaltrust.core.ingest import load
with mock.patch("evaltrust.core.ingest._STREAM_THRESHOLD", 0):
    load("f2_min.json")

Observed on this head:

• without ijson: warning, fallback to read_text + json.loads, load succeeds (generic, 1 example, models A/B).
• with ijson 3.5.1: ValueError: No (example, model, score) records could be extracted.

Root cause: ijson yields Decimal for numbers; coerce_score only accepts int/float/bool/str, so every score row is skipped. Installing the library the warning recommends can break files that load fine without it.

JSON streaming is marked optional in the issue, so deferring JSON (JSONL/CSV only in this PR) or coercing Decimal before dicts_to_records would close the fork. The silent full-load fallback without ijson is a separate honesty point against the "peak bounded by buffer" claim for .json.

Not reviewed

CLI audit end-to-end, multi-metric suite edge cases beyond dual-path equality, adapter correctness outside line detection, and any runtime beyond the benches above. Error text / line numbers for JSONL (blank lines, CRLF, U+2028, empty file) matched main and matched across forced stream vs in-memory paths on this head. Dual-path EvalData equality held for the JSONL/CSV fixtures I tried. load_suite matched load on those checks.

I looked at the streaming path and memory claims only.

1. Peak memory still scales with row count

Issue acceptance asks for peak memory bounded by the streaming buffer, not file size. The generators do avoid Path.read_text() for large JSONL/CSV. That removes the full-file string. Peak still scales with row count because the stream is re-materialised:

    rows = list(row_iter)
    ...
    records = dicts_to_records(rows, skipped)

Same pattern for CSV (rows = list(_iter_csv_rows(...))) and for ijson.items → list(...).

Measured on head 1ade634 vs main 9aebe24, separate processes, macOS ru_maxrss in MiB:
File Path File MiB tracemalloc peak MiB max RSS MiB
padded JSONL (200k ex, pad=400) main load 171 698 1299
same PR load 171 385 1071
compact JSONL (900k ex, no pad) main load 70 1799 4161
same PR load 70 1729 4284

String elimination is real on the padded file (~313 MiB tracemalloc, ~228 MiB RSS). On the compact dump the peak barely moves and stays multi-GiB for a 70 MiB file. So larger-than-RAM audit is still not met for object-heavy score dumps.

Possible direction: convert each row to a Record inside the iteration (or in small batches) so the full list[dict] never coexists with the record list. Longer term, that needs detect_line_adapter / dicts_to_records to accept a one-row lookahead instead of a full list. Until then it may be clearer to state the win as "no full-file string for large JSONL/CSV" rather than "peak bounded by the streaming buffer".

2. Optional ijson changes large-JSON success, not only memory

ijson is not in pyproject.toml dependencies or optional-deps. For a large .json top-level array, the path forks on whether it happens to be installed.

Minimal repro (force the large path with _STREAM_THRESHOLD = 0):

# f2_min.json = [{"id":"q1","model":"A","score":1.0},{"id":"q1","model":"B","score":0.0}]
from unittest import mock
from evaltrust.core.ingest import load
with mock.patch("evaltrust.core.ingest._STREAM_THRESHOLD", 0):
    load("f2_min.json")

Observed on this head:

* **without** `ijson`: warning, fallback to `read_text` + `json.loads`, `load` succeeds (`generic`, 1 example, models A/B).

* **with** `ijson` 3.5.1: `ValueError: No (example, model, score) records could be extracted`.

Root cause: ijson yields Decimal for numbers; coerce_score only accepts int/float/bool/str, so every score row is skipped. Installing the library the warning recommends can break files that load fine without it.

JSON streaming is marked optional in the issue, so deferring JSON (JSONL/CSV only in this PR) or coercing Decimal before dicts_to_records would close the fork. The silent full-load fallback without ijson is a separate honesty point against the "peak bounded by buffer" claim for .json.

Not reviewed

CLI audit end-to-end, multi-metric suite edge cases beyond dual-path equality, adapter correctness outside line detection, and any runtime beyond the benches above. Error text / line numbers for JSONL (blank lines, CRLF, U+2028, empty file) matched main and matched across forced stream vs in-memory paths on this head. Dual-path EvalData equality held for the JSONL/CSV fixtures I tried. load_suite matched load on those checks.

Thanks again for the detailed benchmarks — they directly shaped the follow-up work.

1. Peak memory still scales with row count

Fixed in the latest commit. We added _stream_records_from_jsonl() which peeks the first row to detect layout and — for the common long-format case (model + score columns) — extracts records row-by-row without ever building the full row list. The full list[dict] materialisation is now only the fallback for wide-format and preference files, where _known_models and _is_preference_column genuinely require a full scan.

Measured locally on long-format JSONL with large payload fields:

The module docstring has also been updated to state the guarantee accurately: "no full-file string for JSONL/CSV" for the primary long-format case, with wide-format noted as a fallback.

2. Optional ijson, Decimal issue, and JSON fallback

All three addressed:

• _normalize_ijson_dict() converts Decimal → float at the boundary — your exact repro (f2_min.json with _STREAM_THRESHOLD=0) now loads cleanly with ijson 3.5.1
• ijson added as streaming optional extra in pyproject.toml — pip install 'evaltrust[streaming]'
• Module docstring now explicitly states the memory guarantee applies to JSONL/CSV unconditionally, and to JSON only when ijson is installed

Not reviewed items — noted, out of scope for this PR.

@Maqbool61 thanks for the really rigorous back-and-forth on this — the benchmarks and the honest write-up made the memory picture much clearer, and the latest commit addresses the big things well. Specifically: _stream_records_from_jsonl making the common long-format path genuinely row-by-row (nice ~0.21× on payload-heavy rows), the _normalize_ijson_dict Decimal→float fix (which closes the "installing ijson breaks scores" trap), the streaming optional extra, and tempering the docstring to "no full-file string for JSONL/CSV." That's a solid, honest, partial win.

A few things to close before merge:

1. Align the PR description with the docstring. The docstring is now accurate, but the PR summary at the top still frames this as making it possible to "audit files larger than available RAM." Since records/EvalData are still O(n) and run_audit needs them all resident, that isn't quite true (and your compact-dump benchmark shows it). Could you reword the summary to match the docstring's honest framing — "eliminates the full-file string for large JSONL/CSV," with JSON bounded only when ijson is present?
2. Confirm the multi-metric JSON suite path. Your dual-path equality tests cover JSONL/CSV, but I don't see one for a multi-metric .json file going through the streamed _suite_from_json_streamed path vs the small-file _suite_from_json. Could you add a test asserting the two produce identical suites for a multi-metric JSON (both the top-level array and {"examples": [...]} shapes)? Just want to be sure the streamed path doesn't collapse metrics.
3. Drop the version bump + add a changelog entry. pyproject.toml bumps the version, but that's owned by the release PR (and main is already 0.7.0). Please revert that line and add a CHANGELOG entry under Unreleased describing the streaming ingestion instead.

Non-blocking follow-up: the four ijson routing blocks and _process_row re-implementing dicts_to_records are a fair bit of duplication — worth a later refactor so the streamed and in-memory extraction can't drift, but not needed for this PR.

Once 1–3 are in, this should be good to merge. Really appreciate the depth here.

@Maqbool61 thanks for the really rigorous back-and-forth on this — the benchmarks and the honest write-up made the memory picture much clearer, and the latest commit addresses the big things well. Specifically: _stream_records_from_jsonl making the common long-format path genuinely row-by-row (nice ~0.21× on payload-heavy rows), the _normalize_ijson_dict Decimal→float fix (which closes the "installing ijson breaks scores" trap), the streaming optional extra, and tempering the docstring to "no full-file string for JSONL/CSV." That's a solid, honest, partial win.

A few things to close before merge:

1. Align the PR description with the docstring. The docstring is now accurate, but the PR summary at the top still frames this as making it possible to "audit files larger than available RAM." Since records/EvalData are still O(n) and run_audit needs them all resident, that isn't quite true (and your compact-dump benchmark shows it). Could you reword the summary to match the docstring's honest framing — "eliminates the full-file string for large JSONL/CSV," with JSON bounded only when ijson is present?

2. Confirm the multi-metric JSON suite path. Your dual-path equality tests cover JSONL/CSV, but I don't see one for a multi-metric .json file going through the streamed _suite_from_json_streamed path vs the small-file _suite_from_json. Could you add a test asserting the two produce identical suites for a multi-metric JSON (both the top-level array and {"examples": [...]} shapes)? Just want to be sure the streamed path doesn't collapse metrics.

3. Drop the version bump + add a changelog entry. pyproject.toml bumps the version, but that's owned by the release PR (and main is already 0.7.0). Please revert that line and add a CHANGELOG entry under Unreleased describing the streaming ingestion instead.

Non-blocking follow-up: the four ijson routing blocks and _process_row re-implementing dicts_to_records are a fair bit of duplication — worth a later refactor so the streamed and in-memory extraction can't drift, but not needed for this PR.

Once 1–3 are in, this should be good to merge. Really appreciate the depth here.

@k-dickinson all three items are now addressed in the latest commit (3b363aa):

1. PR description updated — removed the "audit files larger than available RAM" framing. The summary now matches the docstring: "eliminates the full-file string for large JSONL/CSV," with JSON bounded only when ijson is present. The honest scope note (records/EvalData still O(n) in row count) is called out explicitly.

2. Multi-metric JSON suite test added — TestMultiMetricJsonSuite in tests/test_ingest_streaming.py asserts that load_suite() via the streamed path and the small-file path produce identical suites for a multi-metric top-level array JSON file. A third test confirms the streamed path returns both correctness and safety metrics and does not collapse them. All 3 pass.

3. Version bump reverted + CHANGELOG entry added — pyproject.toml and CITATION.cff are back to 0.6.0. A new entry under ## [Unreleased] in CHANGELOG.md describes the streaming ingestion with the same honest framing as the docstring.

Full suite: 790 passed, 0 failed.

This is in good shape — all three items from the last round are addressed, the long-format path is genuinely row-by-row, and I confirmed the streamed extraction matches the in-memory dicts_to_records behaviour. CI is green and the dual-path tests are a nice safety net.

One thing to fix before merge: the branch sets version = "0.6.0" in both pyproject.toml and CITATION.cff, but main is already 0.7.0 (0.7.0 shipped since you branched). Merging as-is would downgrade the version. Can you restore both back to 0.7.0 — the version bump belongs to the release PR, so this branch shouldn't touch those lines at all. Also pyproject.toml lost its trailing newline; please add it back.

Once those are in it's good to merge. Thanks for the depth on this one.

This is in good shape — all three items from the last round are addressed, the long-format path is genuinely row-by-row, and I confirmed the streamed extraction matches the in-memory dicts_to_records behaviour. CI is green and the dual-path tests are a nice safety net.

One thing to fix before merge: the branch sets version = "0.6.0" in both pyproject.toml and CITATION.cff, but main is already 0.7.0 (0.7.0 shipped since you branched). Merging as-is would downgrade the version. Can you restore both back to 0.7.0 — the version bump belongs to the release PR, so this branch shouldn't touch those lines at all. Also pyproject.toml lost its trailing newline; please add it back.

Once those are in it's good to merge. Thanks for the depth on this one.

@k-dickinson done — both pyproject.toml and CITATION.cff are restored to 0.7.0, and the trailing newline is back. Ready to merge.